Re: Potential out-of-bounds access in drivers/scsi/sd.c

[Hannes Reinecke <hare@suse.de>]

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html