On 09/04/2013 05:42 PM, Alan Stern wrote:
> On Wed, 4 Sep 2013, Paolo Bonzini wrote:
> 
>>> --- usb-3.11.orig/drivers/scsi/sd.c
>>> +++ usb-3.11/drivers/scsi/sd.c
>>> @@ -2419,7 +2419,7 @@ sd_read_cache_type(struct scsi_disk *sdk
>>>                     }
>>>             }
>>>  
>>> -           if (modepage == 0x3F) {
>>> +           if (modepage == 0x3F || offset + 2 >= len) {
>>>                     sd_printk(KERN_ERR, sdkp, "No Caching mode page "
>>>                               "present\n");
>>>                     goto defaults;
>>
>> If you do this, the buggy "if" becomes dead code (the loop above doesn't
>> have any "break", so you know that offset >= len and the new condition
>> is always true).
>>
>> So the patch does indeed prevent the bug, but the code can be simplified.
> 
> That's right.  I didn't realize it at first, but the only way to get 
> here is if the next page offset lies beyond the end of the data in the 
> buffer.  Therefore the patch can be simplified as follows.
> 
Oh, pretty please. I already had customers complaining about the
'got wrong page' message. Which again demonstrated nicely the
uncertainty relation between correctness and usability :-)

> Alan Stern
> 
> 
> 
> Index: usb-3.11/drivers/scsi/sd.c
> ===================================================================
> --- usb-3.11.orig/drivers/scsi/sd.c
> +++ usb-3.11/drivers/scsi/sd.c
> @@ -2419,14 +2419,9 @@ sd_read_cache_type(struct scsi_disk *sdk
>                       }
>               }
>  
> -             if (modepage == 0x3F) {
> -                     sd_printk(KERN_ERR, sdkp, "No Caching mode page "
> -                               "present\n");
> -                     goto defaults;
> -             } else if ((buffer[offset] & 0x3f) != modepage) {
> -                     sd_printk(KERN_ERR, sdkp, "Got wrong page\n");
> -                     goto defaults;
> -             }
> +             sd_printk(KERN_ERR, sdkp, "No Caching mode page found\n");
> +             goto defaults;
> +
>       Page_found:
>               if (modepage == 8) {
>                       sdkp->WCE = ((buffer[offset + 2] & 0x04) != 0);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Hannes Reinecke <h...@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                   zSeries & Storage
h...@suse.de                          +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to