Bug 920687

Summary: "sginfo -l" should not open /dev/snapshot
Product: Red Hat Enterprise Linux 6 Reporter: J.H.M. Dassen (Ray) <rdassen>
Component: sg3_utilsAssignee: Dan Horák <dhorak>
Status: CLOSED ERRATA QA Contact: Jan Kepler <jkejda>
Severity: medium Docs Contact:
Priority: high    
Version: 6.4CC: bmr, jkejda, mkarg, rbinkhor, rvdwees
Target Milestone: rcKeywords: EasyFix, Patch
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: sg3_utils-1.28-5.el6 Doc Type: Bug Fix
Doc Text:
Cause: The logic used in the show_devices() function (as used by "sginfo") to identify the available storage devices opens devices rather indiscriminately. Consequence: /dev/snapshot was opened, causing the system to prepare for suspend/hibernate and (as part of that) to block hot-added CPUs from being activated. Fix: The logic used in the show_devices() function to decide which devices to open was adjusted. Result: Running "sginfo -l" no longer has the unintended side effect of blocking the activation of hot-added CPUs.
 Story Points: ---
Clone Of:
: 1020324 (view as bug list) Environment:
Last Closed: 2013-06-19 13:48:25 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 835616, 1020324    
Attachments:
Description Flags
Patch to prevent /dev/snapshot from being opened none

Description J.H.M. Dassen (Ray) 2013-03-12 14:43:45 UTC 
Description of problem:
"sginfo -l" opens /dev/snapshot which is unfortunately not a side effects free operation (cf. bug #920681). sg3_utils should not touch this device.

Version-Release number of selected component (if applicable):
1.28-4.el6

How reproducible:
100%

Steps to Reproduce:
1. strace -f -s128 -e file sginfo -l
  
Actual results:
[...]
2163  open("/dev", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
2163  open("/dev/stderr", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/stdout", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/stdin", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/systty", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/network_throughput", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/network_latency", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/nvram", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/snapshot", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/null", O_RDONLY|O_NONBLOCK) = 4
2163  open("/dev/sg0", O_RDWR|O_NONBLOCK) = -1 ENOENT (No such file or directory)
[...]

Expected results:
Ideally: "sginfo -l" does not attempt to open non-storage devices.
Realistically: "sginfo -l" does not attempt to open /dev/snapshot .

Additional info:
The show_devices() function in src/sginfo.c loops over the entries in /dev, opening those that are character or block devices whose name starts with "n" or "s", opens them and attempts to obtain storage-related information from them.
Comment 2 J.H.M. Dassen (Ray) 2013-04-09 10:42:26 UTC 
Created attachment 733138 [details]
Patch to prevent /dev/snapshot from being opened
Comment 3 J.H.M. Dassen (Ray) 2013-04-09 10:55:14 UTC 
Simple patch added, intended to achieve the realistically expected result.

Changing show_devices() to use sysfs entries to identify storage devices would be a lot cleaner (aligned with the ideally expected result), but also a much more sizeable change.
Comment 6 Dan Horák 2013-04-15 09:15:28 UTC 
(In reply to comment #3)
> Simple patch added, intended to achieve the realistically expected result.
> 
> Changing show_devices() to use sysfs entries to identify storage devices
> would be a lot cleaner (aligned with the ideally expected result), but also
> a much more sizeable change.

for the change to use sysfs please talk to the sg3_utils upstream directly, making this bug and the related kernel one public would also help him
Comment 7 J.H.M. Dassen (Ray) 2013-04-15 09:54:55 UTC 
Uncloaking this bugzilla - Engineering requested this be opened up to facilitate communication with upstream and Global Support Services has not identified any sensitive data in or attached to this bugzilla.
Comment 8 Dan Horák 2013-04-17 08:54:45 UTC 
Reply from upstream author for the record -

sginfo has been in legacy support mode for about 10 years
as far as I'm concerned. Perhaps I should state that more
clearly. There a very few things that it does, that my
newer utilities don't do better and in a cleaner way (e.g.
listing defect blocks is one thing sginfo does that none of
my other utilities do, but that is probably only useful to
the disk manufacturers (or someone trying to reverse
engineer a disk)). sginfo was a quick hack conversion of
scsiinfo to test sg devices circa 1998. Linux 2.0 vintage,
with no devfs, sysfs (and thus lsscsi). A system with 20 SCSI
devices was huge ...

But it doesn't change the fact we can and will apply the fix in the package.
Comment 13 errata-xmlrpc 2013-06-19 13:48:25 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0956.html