[Re: Linux 2.6.26-rc2] Write protect on on

[Re: Linux 2.6.26-rc2] Write protect on on

[Maciej Rutecki]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 3177 bytes --]

I have external drive connected via USB to computer. In 2.6.25.4 whenI plug into computer works fine, and have this messages in dmesg:usb 5-1: new high speed USB device using ehci_hcd and address 4usb 5-1: configuration #1 chosen from 1 choiceInitializing USB Mass Storage driver...scsi2 : SCSI emulation for USB Mass Storage devicesusbcore: registered new interface driver usb-storageUSB Mass Storage support registered.usb-storage: device found at 4usb-storage: waiting for device to settle before scanningscsi 2:0:0:0: Direct-Access     Initio   MHV2080BH PL     3.01 PQ: 0 ANSI: 0usb-storage: device scan completeDriver 'sd' needs updating - please use bus_type methodssd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is offsd 2:0:0:0: [sda] Mode Sense: 00 00 00 00sd 2:0:0:0: [sda] Assuming drive cache: write throughsd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is offsd 2:0:0:0: [sda] Mode Sense: 00 00 00 00sd 2:0:0:0: [sda] Assuming drive cache: write through sda: sda1sd 2:0:0:0: [sda] Attached SCSI disk
Works fine:maciek@zlom:~/kernel.org/2.6.25.4$ ntfsmount /dev/sda1 ~/mnt/ntfs/maciek@zlom:~/kernel.org/2.6.25.4$ mount | grep fusefusectl on /sys/fs/fuse/connections type fusectl (rw)/dev/sda1 on /home/maciek/mnt/ntfs type fuseblk(rw,nosuid,nodev,default_permissions,allow_other,blksize=4096,user=maciek)
But on 2.6.26-rc* sometimes I have messages "Write protect is on", e.g.:sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is onsd 2:0:0:0: [sda] Mode Sense: 09 50 f8 afsd 2:0:0:0: [sda] Assuming drive cache: write throughsd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is offsd 2:0:0:0: [sda] Mode Sense: 00 00 00 00sd 2:0:0:0: [sda] Assuming drive cache: write through sda: sda1sd 2:0:0:0: [sda] Attached SCSI disk
or:sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is onsd 2:0:0:0: [sda] Mode Sense: 09 50 f8 afsd 2:0:0:0: [sda] Assuming drive cache: write throughsd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)sd 2:0:0:0: [sda] Write Protect is onsd 2:0:0:0: [sda] Mode Sense: 09 50 f8 afsd 2:0:0:0: [sda] Assuming drive cache: write through sda: sda1sd 2:0:0:0: [sda] Attached SCSI disk
Then mount fails:maciek@zlom:~/kernel.org/2.6.26-rc2$ ntfsmount /dev/sda1 ~/mnt/ntfs/Error opening partition device: System plików wyłącznie do odczytu.Failed to startup volume: System plików wyłącznie do odczytu.Failed to mount '/dev/sda1': System plików wyłącznie do odczytu.Mount failed.
"System plików wyłącznie do odczytu." means "filesystem readonly"
Tested on two computers PC and laptop.
Dmesg and config:2.6.25.4: http://unixy.pl/maciek/download/kernel/2.6.25.4/2.6.26-rc1: http://unixy.pl/maciek/download/kernel/2.6.26-rc1/2.6.26-rc2: http://unixy.pl/maciek/download/kernel/2.6.26-rc2/
-- Maciej Ruteckihttp://www.maciek.unixy.plÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Alan Stern]

On Fri, 16 May 2008, Maciej Rutecki wrote:

> I have external drive connected via USB to computer. In 2.6.25.4 when
> I plug into computer works fine, and have this messages in dmesg:
> usb 5-1: new high speed USB device using ehci_hcd and address 4
> usb 5-1: configuration #1 chosen from 1 choice
> Initializing USB Mass Storage driver...
> scsi2 : SCSI emulation for USB Mass Storage devices
> usbcore: registered new interface driver usb-storage
> USB Mass Storage support registered.
> usb-storage: device found at 4
> usb-storage: waiting for device to settle before scanning
> scsi 2:0:0:0: Direct-Access     Initio   MHV2080BH PL     3.01 PQ: 0 ANSI: 0
> usb-storage: device scan complete
> Driver 'sd' needs updating - please use bus_type methods
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is off
> sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
> sd 2:0:0:0: [sda] Assuming drive cache: write through
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is off
> sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
> sd 2:0:0:0: [sda] Assuming drive cache: write through
>  sda: sda1
> sd 2:0:0:0: [sda] Attached SCSI disk
> 
> Works fine:
> maciek@zlom:~/kernel.org/2.6.25.4$ ntfsmount /dev/sda1 ~/mnt/ntfs/
> maciek@zlom:~/kernel.org/2.6.25.4$ mount | grep fuse
> fusectl on /sys/fs/fuse/connections type fusectl (rw)
> /dev/sda1 on /home/maciek/mnt/ntfs type fuseblk
> (rw,nosuid,nodev,default_permissions,allow_other,blksize=4096,user=maciek)
> 
> But on 2.6.26-rc* sometimes I have messages "Write protect is on", e.g.:
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is on
> sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
> sd 2:0:0:0: [sda] Assuming drive cache: write through
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is off
> sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
> sd 2:0:0:0: [sda] Assuming drive cache: write through
>  sda: sda1
> sd 2:0:0:0: [sda] Attached SCSI disk
> 
> or:
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is on
> sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
> sd 2:0:0:0: [sda] Assuming drive cache: write through
> sd 2:0:0:0: [sda] 156301488 512-byte hardware sectors (80026 MB)
> sd 2:0:0:0: [sda] Write Protect is on
> sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
> sd 2:0:0:0: [sda] Assuming drive cache: write through
>  sda: sda1
> sd 2:0:0:0: [sda] Attached SCSI disk
> 
> Then mount fails:
> maciek@zlom:~/kernel.org/2.6.26-rc2$ ntfsmount /dev/sda1 ~/mnt/ntfs/
> Error opening partition device: System plików wyłącznie do odczytu.
> Failed to startup volume: System plików wyłącznie do odczytu.
> Failed to mount '/dev/sda1': System plików wyłącznie do odczytu.
> Mount failed.
> 
> "System plików wyłącznie do odczytu." means "filesystem readonly"
> 
> Tested on two computers PC and laptop.
> 
> Dmesg and config:
> 2.6.25.4: http://unixy.pl/maciek/download/kernel/2.6.25.4/
> 2.6.26-rc1: http://unixy.pl/maciek/download/kernel/2.6.26-rc1/
> 2.6.26-rc2: http://unixy.pl/maciek/download/kernel/2.6.26-rc2/

The best way to gather more data is to use usbmon and trace what
happens when you plug in the drive (see Documentation/usb/usbmon.txt).  
If you collect traces under both 2.6.25.4 and 2.6.26-rc2 then we can
compare them to see what has changed.

You don't have to trace the mount command; the activity at plug-in time
will be sufficient.

Alan Stern

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Maciej Rutecki]

2008/5/17 Alan Stern <stern@rowland.harvard.edu>:

> The best way to gather more data is to use usbmon and trace what
> happens when you plug in the drive (see Documentation/usb/usbmon.txt).
> If you collect traces under both 2.6.25.4 and 2.6.26-rc2 then we can
> compare them to see what has changed.
>
> You don't have to trace the mount command; the activity at plug-in time
> will be sufficient.
>
> Alan Stern
>
>

2.6.25.4 (works fine):
http://unixy.pl/maciek/download/kernel/2.6.25.4/syslog_debug.txt
http://unixy.pl/maciek/download/kernel/2.6.25.4/usbmon.txt

2.6.26-rc2 ("write protect is on" problem; can't mount device):
http://unixy.pl/maciek/download/kernel/2.6.26-rc2/syslog_debug.txt
http://unixy.pl/maciek/download/kernel/2.6.26-rc2/usbmon.txt

-- 
Maciej Rutecki
http://www.maciek.unixy.pl

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Alan Stern]

Summary: 2.6.26-rc2 doesn't detect a USB drive's write-protect setting 
correctly.

On Sat, 17 May 2008, Maciej Rutecki wrote:

> 2.6.25.4 (works fine):
> http://unixy.pl/maciek/download/kernel/2.6.25.4/syslog_debug.txt
> http://unixy.pl/maciek/download/kernel/2.6.25.4/usbmon.txt
> 
> 2.6.26-rc2 ("write protect is on" problem; can't mount device):
> http://unixy.pl/maciek/download/kernel/2.6.26-rc2/syslog_debug.txt
> http://unixy.pl/maciek/download/kernel/2.6.26-rc2/usbmon.txt

I'm not sure exactly what changed to cause this regression, but the 
problem lies in the SCSI layer, not the USB layer.

The logs show that in response to the 192-byte MODE SENSE command (used
to read the write-protect status), the device sends back no data, good
status, and Residue = 192.  The SCSI core ignores the Residue and
thinks that the old left-over data in the buffer (in this case left
over from the READ CAPACITY command) actually indicates the
write-protect status -- which it obviously doesn't.

Boaz, is scsi_mode_sense() the right place to check for this sort of 
thing?  It probably should be treated the same as an Illegal Request 
error.

Alan Stern

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Boaz Harrosh]

Alan Stern wrote:
> Summary: 2.6.26-rc2 doesn't detect a USB drive's write-protect setting 
> correctly.
> 
> On Sat, 17 May 2008, Maciej Rutecki wrote:
> 
>> 2.6.25.4 (works fine):
>> http://unixy.pl/maciek/download/kernel/2.6.25.4/syslog_debug.txt
>> http://unixy.pl/maciek/download/kernel/2.6.25.4/usbmon.txt
>>
>> 2.6.26-rc2 ("write protect is on" problem; can't mount device):
>> http://unixy.pl/maciek/download/kernel/2.6.26-rc2/syslog_debug.txt
>> http://unixy.pl/maciek/download/kernel/2.6.26-rc2/usbmon.txt
> 
> I'm not sure exactly what changed to cause this regression, but the 
> problem lies in the SCSI layer, not the USB layer.
> 
> The logs show that in response to the 192-byte MODE SENSE command (used
> to read the write-protect status), the device sends back no data, good
> status, and Residue = 192.  The SCSI core ignores the Residue and
> thinks that the old left-over data in the buffer (in this case left
> over from the READ CAPACITY command) actually indicates the
> write-protect status -- which it obviously doesn't.
> 
> Boaz, is scsi_mode_sense() the right place to check for this sort of 
> thing?  It probably should be treated the same as an Illegal Request 
> error.
> 
> Alan Stern
> 

Do you mean this diff below:

@@ -796,133 +789,133 @@ kernel: usb-storage: *** thread awakened
 kernel: usb-storage: Command MODE_SENSE (6 bytes)
 kernel: usb-storage:  1a 00 3f 00 c0 00
 kernel: usb-storage: Bulk Command S 0x43425355 T 0x4 L 192 F 128 Trg 0 LUN 0 CL 6
 kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
 kernel: usb-storage: Status code 0; transferred 31/31
 kernel: usb-storage: -- transfer complete
 kernel: usb-storage: Bulk command transfer result=0
 kernel: usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
 kernel: usb-storage: Status code -32; transferred 0/192
 kernel: usb-storage: clearing endpoint halt for pipe 0xc0008480
 kernel: usb-storage: usb_stor_control_msg: rq=01 rqtype=02 value=0000 index=81 len=0
 kernel: usb-storage: usb_stor_clear_halt: result = 0
 kernel: usb-storage: Bulk data transfer result 0x2
 kernel: usb-storage: Attempting to get CSW...
 kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
 kernel: usb-storage: Status code 0; transferred 13/13
 kernel: usb-storage: -- transfer complete
 kernel: usb-storage: Bulk status result = 0
 kernel: usb-storage: Bulk Status S 0x53425355 T 0x4 R 192 Stat 0x0
 kernel: usb-storage: scsi cmd done, result=0x0
 kernel: usb-storage: *** thread sleeping.
-kernel: sd 2:0:0:0: [sda] Write Protect is off
-kernel: sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
+kernel: sd 2:0:0:0: [sda] Write Protect is on
+kernel: sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
 kernel: sd 2:0:0:0: [sda] Assuming drive cache: write through
 kernel: usb-storage: queuecommand called

("+" is the new kernel and "-" the older one)

It looks like it used to be the same exact return and everything only that at
old kernel the 4 bytes used to be zero and now they are not.

So It looks to me that it never used to work (Data was never actually read
from device) but by luck, the garbage data used to be a better default 
"Write Protect is off"

I do not think it is legal in scsi to return "Nothing was read" with no
error condition. You are probably right that we do not at all check resid
if status is 0, even though short reads are allowed with out error status
in some cases, as per command. But this is not the case here here nothing
was read at all, status must be returned. Or even worse if this command 
is mandatory by scsi but not supported by some USB devices then it will
have to be emulated by usb_storage.

My $0.017
Boaz

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Alan Stern]

On Sun, 18 May 2008, Boaz Harrosh wrote:

> Do you mean this diff below:
> 
> @@ -796,133 +789,133 @@ kernel: usb-storage: *** thread awakened
>  kernel: usb-storage: Command MODE_SENSE (6 bytes)
>  kernel: usb-storage:  1a 00 3f 00 c0 00
>  kernel: usb-storage: Bulk Command S 0x43425355 T 0x4 L 192 F 128 Trg 0 LUN 0 CL 6
>  kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
>  kernel: usb-storage: Status code 0; transferred 31/31
>  kernel: usb-storage: -- transfer complete
>  kernel: usb-storage: Bulk command transfer result=0
>  kernel: usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
>  kernel: usb-storage: Status code -32; transferred 0/192
>  kernel: usb-storage: clearing endpoint halt for pipe 0xc0008480
>  kernel: usb-storage: usb_stor_control_msg: rq=01 rqtype=02 value=0000 index=81 len=0
>  kernel: usb-storage: usb_stor_clear_halt: result = 0
>  kernel: usb-storage: Bulk data transfer result 0x2
>  kernel: usb-storage: Attempting to get CSW...
>  kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
>  kernel: usb-storage: Status code 0; transferred 13/13
>  kernel: usb-storage: -- transfer complete
>  kernel: usb-storage: Bulk status result = 0
>  kernel: usb-storage: Bulk Status S 0x53425355 T 0x4 R 192 Stat 0x0
>  kernel: usb-storage: scsi cmd done, result=0x0
>  kernel: usb-storage: *** thread sleeping.
> -kernel: sd 2:0:0:0: [sda] Write Protect is off
> -kernel: sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
> +kernel: sd 2:0:0:0: [sda] Write Protect is on
> +kernel: sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
>  kernel: sd 2:0:0:0: [sda] Assuming drive cache: write through
>  kernel: usb-storage: queuecommand called
> 
> ("+" is the new kernel and "-" the older one)

That's right.

> It looks like it used to be the same exact return and everything only that at
> old kernel the 4 bytes used to be zero and now they are not.
> 
> So It looks to me that it never used to work (Data was never actually read
> from device) but by luck, the garbage data used to be a better default 
> "Write Protect is off"

Yes, it never worked properly.  But now it fails in a bad way whereas 
before it failed in a benign way.

> I do not think it is legal in scsi to return "Nothing was read" with no
> error condition.

I'm not aware of any part of the spec where it says that.  In any case 
it doesn't matter what the spec says; we ought to be able to drive this 
device even if it isn't compliant with the spec.

> You are probably right that we do not at all check resid
> if status is 0, even though short reads are allowed with out error status
> in some cases, as per command. But this is not the case here here nothing
> was read at all, status must be returned. Or even worse if this command 
> is mandatory by scsi but not supported by some USB devices then it will
> have to be emulated by usb_storage.

The real question is how should we fix the problem.  For the sake of 
argument, let's say that we fix it by changing scsi_mode_sense() -- 
make the routine return 0 if the residue is so large that there isn't 
even a valid header.

But how can this be done?  Should we modify struct scsi_sense_hdr, by 
adding a "residue" field?

Alan Stern

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Boaz Harrosh]

Alan Stern wrote:
> On Sun, 18 May 2008, Boaz Harrosh wrote:
> 
>> Do you mean this diff below:
>>
>> @@ -796,133 +789,133 @@ kernel: usb-storage: *** thread awakened
>>  kernel: usb-storage: Command MODE_SENSE (6 bytes)
>>  kernel: usb-storage:  1a 00 3f 00 c0 00
>>  kernel: usb-storage: Bulk Command S 0x43425355 T 0x4 L 192 F 128 Trg 0 LUN 0 CL 6
>>  kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 31 bytes
>>  kernel: usb-storage: Status code 0; transferred 31/31
>>  kernel: usb-storage: -- transfer complete
>>  kernel: usb-storage: Bulk command transfer result=0
>>  kernel: usb-storage: usb_stor_bulk_transfer_sglist: xfer 192 bytes, 1 entries
>>  kernel: usb-storage: Status code -32; transferred 0/192
>>  kernel: usb-storage: clearing endpoint halt for pipe 0xc0008480
>>  kernel: usb-storage: usb_stor_control_msg: rq=01 rqtype=02 value=0000 index=81 len=0
>>  kernel: usb-storage: usb_stor_clear_halt: result = 0
>>  kernel: usb-storage: Bulk data transfer result 0x2
>>  kernel: usb-storage: Attempting to get CSW...
>>  kernel: usb-storage: usb_stor_bulk_transfer_buf: xfer 13 bytes
>>  kernel: usb-storage: Status code 0; transferred 13/13
>>  kernel: usb-storage: -- transfer complete
>>  kernel: usb-storage: Bulk status result = 0
>>  kernel: usb-storage: Bulk Status S 0x53425355 T 0x4 R 192 Stat 0x0
>>  kernel: usb-storage: scsi cmd done, result=0x0
>>  kernel: usb-storage: *** thread sleeping.
>> -kernel: sd 2:0:0:0: [sda] Write Protect is off
>> -kernel: sd 2:0:0:0: [sda] Mode Sense: 00 00 00 00
>> +kernel: sd 2:0:0:0: [sda] Write Protect is on
>> +kernel: sd 2:0:0:0: [sda] Mode Sense: 09 50 f8 af
>>  kernel: sd 2:0:0:0: [sda] Assuming drive cache: write through
>>  kernel: usb-storage: queuecommand called
>>
>> ("+" is the new kernel and "-" the older one)
> 
> That's right.
> 
>> It looks like it used to be the same exact return and everything only that at
>> old kernel the 4 bytes used to be zero and now they are not.
>>
>> So It looks to me that it never used to work (Data was never actually read
>> from device) but by luck, the garbage data used to be a better default 
>> "Write Protect is off"
> 
> Yes, it never worked properly.  But now it fails in a bad way whereas 
> before it failed in a benign way.
> 

You do realize that, that was pure lock to have a zero'ed buffer.

>> I do not think it is legal in scsi to return "Nothing was read" with no
>> error condition.
> 
> I'm not aware of any part of the spec where it says that.  In any case 
> it doesn't matter what the spec says; we ought to be able to drive this 
> device even if it isn't compliant with the spec.
> 

Yes it is so. In read_x command a short read is an error status (or short
writes for write_x). The only commands that are allowed short reads 
(with no error) are those commands that have optional sizes like inquiry
and stuff, so if the device does not have the extra info it will return
a short read, of exact number of bytes as permitted by the shorter version
of the command. The initiator will understand that the command return is of
the shorter version. And also REQUEST_SENSE can be short. But all other 
commands must return the number of bytes requested or set a status. Here
the device does not return a shorter MODE_SENSE, it does not return it
at all. A status must be set/emulated.

>> You are probably right that we do not at all check resid
>> if status is 0, even though short reads are allowed with out error status
>> in some cases, as per command. But this is not the case here here nothing
>> was read at all, status must be returned. Or even worse if this command 
>> is mandatory by scsi but not supported by some USB devices then it will
>> have to be emulated by usb_storage.
> 
> The real question is how should we fix the problem.  For the sake of 
> argument, let's say that we fix it by changing scsi_mode_sense() -- 
> make the routine return 0 if the residue is so large that there isn't 
> even a valid header.
> 
> But how can this be done?  Should we modify struct scsi_sense_hdr, by 
> adding a "residue" field?
> 

I would not do that, because:
- This is a per command filter and will not be easy to do right.
- All other drivers will need to be inspected, because mid-layer
  behavior change. Where before the gate was status, then if set
  look at resid. What you propose enables another gate for errors.
  I'm much more lazy then you, and for my peace of mine will not do
  such a subtle change, that I can never fully test for.

If I where you I would fix it at the usb level, where if *no* data
was read/written I would atleast set the DID_NOT_CONNECT status.
(OK I'm not sure what to do here it should be carefully considered)

Also be ware that I think the MODE_SENSE command is mandatory and
you must emulate it if the device does not supports it. (That is
filter for MODE_SENSE command and if returns error, set a sensible
returned buffer with info gathered by other means. Like good'ol 
isd200 does it)

> Alan Stern
> 

Boaz

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Linus Torvalds]

On Mon, 19 May 2008, Boaz Harrosh wrote:

> Alan Stern wrote:
> > 
> > Yes, it never worked properly.  But now it fails in a bad way whereas 
> > before it failed in a benign way.
> 
> You do realize that, that was pure lock to have a zero'ed buffer.

Umm. Maybe it SHOULD NOT HAVE BEEN!

The thing is, if we can get partial results back, we really *should* 
either error out, or we should have at least cleared the buffer (either 
beforehand or when seeing the partial result). Returning a buffer with the 
old random contents is a bug.

And if clearing the buffer not only avoids any security holes and possible 
undefined behavior, but _also_ ends up fixing the write protect sense 
issue, all the better!

		Linus

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Boaz Harrosh]

Linus Torvalds wrote:
> 
> On Mon, 19 May 2008, Boaz Harrosh wrote:
> 
>> Alan Stern wrote:
>>> Yes, it never worked properly.  But now it fails in a bad way whereas 
>>> before it failed in a benign way.
>> You do realize that, that was pure lock to have a zero'ed buffer.
> 
> Umm. Maybe it SHOULD NOT HAVE BEEN!
> 
> The thing is, if we can get partial results back, we really *should* 
> either error out, or we should have at least cleared the buffer (either 
> beforehand or when seeing the partial result). Returning a buffer with the 
> old random contents is a bug.
> 
> And if clearing the buffer not only avoids any security holes and possible 
> undefined behavior, but _also_ ends up fixing the write protect sense 
> issue, all the better!
> 
> 		Linus
> --

Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
places that initiate a MODE_SENSE command, specifically at
scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)

Boaz

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Linus Torvalds]

On Mon, 19 May 2008, Boaz Harrosh wrote:
> 
> Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
> spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
> places that initiate a MODE_SENSE command, specifically at
> scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)

I was actualyl more thinking that the safest thing to do would be to just 
pre-clear the sense buffer. Then, if some driver doesn't fill it 
correctly, big deal..

It's not like pre-clearing the bugger is a performance issue.

		Linus

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Boaz Harrosh]

Linus Torvalds wrote:
> 
> On Mon, 19 May 2008, Boaz Harrosh wrote:
>> Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
>> spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
>> places that initiate a MODE_SENSE command, specifically at
>> scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)
> 
> I was actualyl more thinking that the safest thing to do would be to just 
> pre-clear the sense buffer. Then, if some driver doesn't fill it 
> correctly, big deal..

The sense buffer *is* always cleared and it is mandated by scsi spec.
But the problem above is that the actual data buffer for read, had 
garbage data, and *no* read was actually preformed do to none-standard 
device response.

> 
> It's not like pre-clearing the bugger is a performance issue.
> 
> 		Linus
> --

Boaz

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Matthew Wilcox]

On Mon, May 19, 2008 at 10:27:17AM -0700, Linus Torvalds wrote:
> On Mon, 19 May 2008, Boaz Harrosh wrote:
> > 
> > Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
> > spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
> > places that initiate a MODE_SENSE command, specifically at
> > scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)
> 
> I was actualyl more thinking that the safest thing to do would be to just 
> pre-clear the sense buffer. Then, if some driver doesn't fill it 
> correctly, big deal..
> 
> It's not like pre-clearing the bugger is a performance issue.

Actually, I think it might be.  I don't have any hard evidence for this
right now, but I'll let you know if I do.  There's certainly a lot of L2
cache misses in the scsi command submission path.

I'm not arguing that speed should trump performance, just we might
want to not clear the sense buffer if we know we have a non-USB device.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[Alan Stern]

On Mon, 19 May 2008, Boaz Harrosh wrote:

> Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
> spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
> places that initiate a MODE_SENSE command, specifically at
> scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)

I can't send such a fix because at these places the residue information 
has already been lost.  The fix needs to be made lower down.


Besides, everybody seems to be missing an important point.

MODE SENSE is just one example of a command for which a device might
return less data than the host expected.  In principle the same thing
could happen with _any_ command.  The host should be prepared for this
and should be able to handle it correctly.  And the host shouldn't
blindly assume that devices will slavishly follow the letter of the
SCSI spec.

We need something much more thorough than just fiddling with
scsi_mode_sense().  One possibility would be to pass a
minimum-response-length argument to scsi_execute_req().  But even that
wouldn't catch all the code paths where this sort of thing could  
happen (although it probably would catch most of them).

This needs someone who is more familiar than I am with the operation of
the SCSI core.
  
Alan Stern

Re: [Re: Linux 2.6.26-rc2] Write protect on on

[James Bottomley]

On Mon, 2008-05-19 at 10:27 -0700, Linus Torvalds wrote:
> 
> On Mon, 19 May 2008, Boaz Harrosh wrote:
> > 
> > Sure, inspecting other places that emulate MODE_SENSE, (And inspecting the scsi
> > spec) all zeros is a very good scsi response. Alan do you want to send a fix for all
> > places that initiate a MODE_SENSE command, specifically at
> > scsi_scan.c::scsi_unlock_floptical() ? (Some other places do)
> 
> I was actualyl more thinking that the safest thing to do would be to just 
> pre-clear the sense buffer. Then, if some driver doesn't fill it 
> correctly, big deal..
> 
> It's not like pre-clearing the bugger is a performance issue.

It already is pre cleared, for precisely the reason that some devices
lie about returning sense, so we use the zero return to mean no sense
was collected.

James