Il 04/09/2013 16:32, Alan Stern ha scritto: > On Wed, 4 Sep 2013, Dmitry Vyukov wrote: > >> Hi, >> >> We are working on a memory error detector AddressSanitizer for Linux >> kernel (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel), >> it can detect use-after-free and buffer-overflow errors. > > ... > >> The code in sd_read_cache_type does the following: >> >> while (offset < len) { >> ... >> } >> ... >> if ((buffer[offset] & 0x3f) != modepage) { >> sd_printk(KERN_ERR, sdkp, "Got wrong page\n"); >> goto defaults; >> } >> >> When control leaves the while loop, offset >= len, so buffer[offset] >> reads random garbage out-of-bounds. >> It the worst case it can lead to crash, or if (buffer[offset] & 0x3f) >> happen to be == modepage, then it will read more garbage. >> >> Please help validate and triage this. > > The tool's output is correct. The patch below should fix it. > > 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,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. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
