aic7(censored) use after free in 2.5.66

aic7(censored) use after free in 2.5.66

[Zwane Mwaikambo]

I got this on boot on an 8way/16G box, perhaps Justin should try 
and push his latest? CONFIG_PREEMPT=y if that makes any difference at 
all.. If anyone is interested i can provide more info.

scsi0 : Adaptec AIC7XXX EISA/VLB/PCI SCSI HBA DRIVER, Rev 6.2.28

Slab corruption: start=f7d66248, expend=f7d662c7, problemat=f7d662ac
Last user: [<c024f0b7>](ahc_linux_free_device+0x27/0x60)
Data: 
****************************************************************************************************6C 
**************************A5 
Next: 71 F0 2C .B7 F0 24 C0 A5 C2 0F 17 00 A0 E8 EB 00 80 85 EC C2 03 00 00 C2 03 00 00 00 00 00 A0 
slab error in check_poison_obj(): cache `size-128': object was modified 
after freeing
Call Trace:
 [<c01436c9>] check_poison_obj+0x179/0x190
 [<c0144f5d>] kmalloc+0xdd/0x190
 [<c01dd4fa>] con_insert_unipair+0x8a/0xe0
 [<c0144f5d>] kmalloc+0xdd/0x190
 [<c01dd5a4>] con_clear_unimap+0x54/0xc0
 [<c0143583>] check_poison_obj+0x33/0x190
 [<c01dd4fa>] con_insert_unipair+0x8a/0xe0
 [<c01dd926>] con_set_default_unimap+0xc6/0x140
 [<c01e0a35>] vc_allocate+0x95/0x130
 [<c01e4039>] con_open+0x19/0x90
 [<c01d2389>] tty_open+0x249/0x460
 [<c0143583>] check_poison_obj+0x33/0x190
 [<c0164e57>] get_chrfops+0x27/0xb0
 [<c015cf97>] get_empty_filp+0x47/0xe0
 [<c0165262>] chrdev_open+0x82/0x100
 [<c015b3e7>] dentry_open+0xc7/0x160
 [<c0144dde>] kmem_cache_alloc+0x9e/0x140
 [<c015b30d>] filp_open+0x4d/0x60
 [<c016933e>] getname+0x5e/0xa0
 [<c015b765>] sys_open+0x35/0x70
 [<c010aecf>] syscall_call+0x7/0xb


-- 
function.linuxpower.ca

Re: aic7(censored) use after free in 2.5.66

[Zwane Mwaikambo]

On Tue, 1 Apr 2003, Zwane Mwaikambo wrote:

> I got this on boot on an 8way/16G box, perhaps Justin should try 
> and push his latest? CONFIG_PREEMPT=y if that makes any difference at 
> all.. If anyone is interested i can provide more info.
> 
> scsi0 : Adaptec AIC7XXX EISA/VLB/PCI SCSI HBA DRIVER, Rev 6.2.28
> 
> Slab corruption: start=f7d66248, expend=f7d662c7, problemat=f7d662ac
> Last user: [<c024f0b7>](ahc_linux_free_device+0x27/0x60)
> Data: 

This probably wants; Or if we can sleep in all the paths, a  
synchronize_kernel after del_timer should suffice.

Justin?

Index: linux-2.5.66/drivers/scsi/aic7xxx/aic7xxx_osm.c
===================================================================
RCS file: /build/cvsroot/linux-2.5.66/drivers/scsi/aic7xxx/aic7xxx_osm.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 aic7xxx_osm.c
--- linux-2.5.66/drivers/scsi/aic7xxx/aic7xxx_osm.c	24 Mar 2003 23:39:44 -0000	1.1.1.1
+++ linux-2.5.66/drivers/scsi/aic7xxx/aic7xxx_osm.c	1 Apr 2003 06:54:01 -0000
@@ -4097,7 +4097,7 @@ ahc_linux_free_device(struct ahc_softc *
 {
 	struct ahc_linux_target *targ;
 
-	del_timer(&dev->timer);
+	del_timer_sync(&dev->timer);
 	targ = dev->target;
 	targ->devices[dev->lun] = NULL;
 	free(dev, M_DEVBUF);
-- 
function.linuxpower.ca

Re: aic7(censored) use after free in 2.5.66

[Andrew Morton]

Zwane Mwaikambo <zwane@linuxpower.ca> wrote:
>
> > Slab corruption: start=f7d66248, expend=f7d662c7, problemat=f7d662ac
> > Last user: [<c024f0b7>](ahc_linux_free_device+0x27/0x60)
> > Data: 
> 
> This probably wants; Or if we can sleep in all the paths, a  
> synchronize_kernel after del_timer should suffice.

Yes, but no.

The corruption was at offset 52 decimal into struct ahc_linux_device. 
Without knowing your config it is hard for me to work out what you have at
that offset.   Rebuild your kernel with -g and do:

(gdb) p/d &(((struct ahc_linux_device *)0)->maxtags)

until you find which member is at offset 52.

Something incremented that field by one after it was freed.

Re: aic7(censored) use after free in 2.5.66

[Zwane Mwaikambo]

On Mon, 31 Mar 2003, Andrew Morton wrote:

> The corruption was at offset 52 decimal into struct ahc_linux_device. 
> Without knowing your config it is hard for me to work out what you have at
> that offset.   Rebuild your kernel with -g and do:
> 
> (gdb) p/d &(((struct ahc_linux_device *)0)->maxtags)
> 
> until you find which member is at offset 52.
> 
> Something incremented that field by one after it was freed.

(gdb) p/d &(((struct ahc_linux_device *)0)->timer.lock)
$4 = 52

That would be a lock free it appears.

-- 
function.linuxpower.ca

Re: aic7(censored) use after free in 2.5.66

[Andrew Morton]

Zwane Mwaikambo <zwane@linuxpower.ca> wrote:
>
> On Mon, 31 Mar 2003, Andrew Morton wrote:
> 
> > The corruption was at offset 52 decimal into struct ahc_linux_device. 
> > Without knowing your config it is hard for me to work out what you have at
> > that offset.   Rebuild your kernel with -g and do:
> > 
> > (gdb) p/d &(((struct ahc_linux_device *)0)->maxtags)
> > 
> > until you find which member is at offset 52.
> > 
> > Something incremented that field by one after it was freed.
> 
> (gdb) p/d &(((struct ahc_linux_device *)0)->timer.lock)
> $4 = 52
> 
> That would be a lock free it appears.

OK, so that's a spin_unlock(&timer->lock) in the timer code itself.  Your
patch will fix that up.

We just need to be sure that the del_timer_sync() is not called while holding
any locks which would prevent the timer handler from completing.  

Re: aic7(censored) use after free in 2.5.66

[Zwane Mwaikambo]

On Mon, 31 Mar 2003, Andrew Morton wrote:

> OK, so that's a spin_unlock(&timer->lock) in the timer code itself.  Your
> patch will fix that up.
> 
> We just need to be sure that the del_timer_sync() is not called while holding
> any locks which would prevent the timer handler from completing.  

Quick audit says we should be ok. I can do a few simple tests on this.

-- 
function.linuxpower.ca

Re: aic7(censored) use after free in 2.5.66

[Zwane Mwaikambo]

On Tue, 1 Apr 2003, Zwane Mwaikambo wrote:

> On Mon, 31 Mar 2003, Andrew Morton wrote:
> 
> > OK, so that's a spin_unlock(&timer->lock) in the timer code itself.  Your
> > patch will fix that up.
> > 
> > We just need to be sure that the del_timer_sync() is not called while holding
> > any locks which would prevent the timer handler from completing.  
> 
> Quick audit says we should be ok. I can do a few simple tests on this.

Ok came up and went down clean.

-- 
function.linuxpower.ca