Re: Memory corruption & SCSI in 2.6.15

Re: Memory corruption & SCSI in 2.6.15

[Brian King]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

Benjamin Herrenschmidt wrote:
> Hi !
> 
> Current -git as of today (that is 2.6.15-rc5 + the batch of fixes Linus
> pulled after his return) was dying in weird ways for me on POWER5. I had
> the good idea to activate slab debugging, and I now see it detecting
> slab corruption as soon as the IPR driver initializes.

Please try the attached patch. There appears to be a double free going on
in the scsi scan code. There is a direct call to scsi_free_queue and then
the following put_device calls the release function, which also frees
the queue.

Brian


> Since I remember seeing a discussion somewhere on a list between Brian
> King and Jens Axboe about use-after-free problems in SCSI and possible
> other niceties of that sort, I though it might be related...
> 
> Anything I can do to help track this down ?
> 
> ipr: IBM Power RAID SCSI Device Driver version: 2.1.0 (October 31, 2005)
> ipr 0000:c0:01.0: Found IOA with IRQ: 99
> ipr 0000:c0:01.0: Starting IOA initialization sequence.
> ipr 0000:c0:01.0: Adapter firmware version: 020A004E
> ipr 0000:c0:01.0: IOA initialized.
> scsi0 : IBM 570B Storage Adapter
> Slab corruption: start=c000000070de39a0, len=728
> Redzone: 0x5a2cf071/0x5a2cf071.
> Last user: [<c0000000002297c4>](.blk_cleanup_queue+0xe4/0x170)
> 1d0: 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 00 00 00 00
> 2b0: 6b 6b 6b 6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Prev obj: start=c000000070de36b0, len=728
> Redzone: 0x5a2cf071/0x5a2cf071.
> Last user: [<0000000000000000>](0x0)
> 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> Next obj: start=c000000070de3c90, len=728
> Redzone: 0x170fc2a5/0x170fc2a5.
> Last user: [<c000000000227b00>](.blk_alloc_queue_node+0x30/0x90)
> 
> Ben.
> 
> 


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

[-- Attachment #2: scsi_scan_use_after_free.patch --]
[-- Type: text/x-patch, Size: 763 bytes --]


Current scsi scanning code appears to have a use after free
bug is a LLDD's slave_alloc fails. Remove the redundant
scsi_free_queue.

Signed-off-by: Brian King <brking@us.ibm.com>
---

 drivers/scsi/scsi_scan.c |    1 -
 1 files changed, 1 deletion(-)

diff -puN drivers/scsi/scsi_scan.c~scsi_scan_use_after_free drivers/scsi/scsi_scan.c
--- linux-2.6/drivers/scsi/scsi_scan.c~scsi_scan_use_after_free	2005-12-12 13:00:28.000000000 -0600
+++ linux-2.6-bjking1/drivers/scsi/scsi_scan.c	2005-12-12 13:00:28.000000000 -0600
@@ -279,7 +279,6 @@ static struct scsi_device *scsi_alloc_sd
 
 out_device_destroy:
 	transport_destroy_device(&sdev->sdev_gendev);
-	scsi_free_queue(sdev->request_queue);
 	put_device(&sdev->sdev_gendev);
 out:
 	if (display_failure_msg)
_

Re: Memory corruption & SCSI in 2.6.15

[Linus Torvalds]

On Mon, 12 Dec 2005, Brian King wrote:
> 
> Please try the attached patch. There appears to be a double free going on
> in the scsi scan code. There is a direct call to scsi_free_queue and then
> the following put_device calls the release function, which also frees
> the queue.

Indeed, that looks pretty subtle. 

James: Brian's patch looks obviously correct to me (scsi_alloc_sdev() will 
have called scsi_sysfs_device_initialize() which will set up the release 
function to free the queue). 

This code has been like that forever, though, which makes me wonder. Can 
anybody see what has changed to make the bug trigger? Or is there 
something I'm missing?

		Linus

Re: Memory corruption & SCSI in 2.6.15

[Nathan Lynch]

Brian King wrote:
> Benjamin Herrenschmidt wrote:
> >Hi !
> >
> >Current -git as of today (that is 2.6.15-rc5 + the batch of fixes Linus
> >pulled after his return) was dying in weird ways for me on POWER5. I had
> >the good idea to activate slab debugging, and I now see it detecting
> >slab corruption as soon as the IPR driver initializes.
> 
> Please try the attached patch. There appears to be a double free going on
> in the scsi scan code. There is a direct call to scsi_free_queue and then
> the following put_device calls the release function, which also frees
> the queue.

Tested against 2.6.15-rc5, seems to fix it, thanks.


Nathan

Re: Memory corruption & SCSI in 2.6.15

[James Bottomley]

On Mon, 2005-12-12 at 11:55 -0800, Linus Torvalds wrote:
> Indeed, that looks pretty subtle. 
> 
> James: Brian's patch looks obviously correct to me (scsi_alloc_sdev() will 
> have called scsi_sysfs_device_initialize() which will set up the release 
> function to free the queue). 

Yes it does ... I'll put it in the rc-fixes tree.

> This code has been like that forever, though, which makes me wonder. Can 
> anybody see what has changed to make the bug trigger? Or is there 
> something I'm missing?

The trigger, based on the failure path has to be a slave_alloc failure
of an underlying driver (which isn't that common).  This may not be
visible in the dmesg traces if anyone has one, because reporting the
condition is up to the driver.

James