Re: Bad module reference counter

[Stanislaw Gruszka <stf_xl@wp.pl>]

Wednesday 18 February 2009 22:25:19 Bartlomiej Zolnierkiewicz napisał(a):
> > I entered a problem with double decreasing module reference counter
> > where it become "negative", here is the usage scenario:
> > 
> > # modprobe at91_ide
> > # modprobe ide_gd_mod
> > # lsmod
> > Module                  Size  Used by    Not tainted
> > ide_gd_mod             22948  0
> > at91_ide                4672  0
> > ide_core               77020  2 ide_gd_mod,at91_ide
> > # rmmod ide_gd_mod
> > # lsmod
> > Module                  Size  Used by    Not tainted
> > at91_ide                4672 4294967295
> > ide_core               77020  1 at91_ide
> > 
> > Note when I first remove at91_ide module and then ide_gd_mod
> > everyting is ok.
> > 
> > I tired to debug issue and I did not found any suspicious in at91_ide.
> > I think probable reason is double free in ide-gd.c . Here is patch with
> > workaround (or maybe it is a real fix, but I'm not sure):
> > 
> > diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
> > index 7857b20..31ae04e 100644
> > --- a/drivers/ide/ide-gd.c
> > +++ b/drivers/ide/ide-gd.c
> > @@ -70,8 +70,6 @@ static void ide_gd_remove(ide_drive_t *drive)
> >  	del_gendisk(g);
> >  
> >  	drive->disk_ops->flush(drive);
> > -
> > -	ide_disk_put(idkp);
> >  }
> >  
> >  static void ide_disk_release(struct kref *kref)
> > 
> > If this patch is ok, maybe similar things need to be done also in ide-cd and
> > perhaps other device type modules.
> 
> Seems like ide_device_put() needs the same module_refcount() check that
> is present in scsi_device_put() so removal of device driver won't trigger
> a spurious module_put() on a host driver?

I little surprise about scsi code (linux-scsi ML CC). Is comment inside
scsi_device_put() function correct? Why scsi_device_get() not check
try_module_get() return value? And most importand: there is reference
counter check before put, so it can be 0, but data does it protect is in
use ?

Adding module_refcount() != 0 to ide_device_put()  helps only partially, below
commands sequence give oops [1].

# modprobe at91_ide
# modprobe ide_gd_mod
# rmmod ide_gd_mod
# modprobe ide_gd_mod
# rmmod at91_ide

Oops happens because previous "rmmod ide_gd_mod"  decrease some reference
counter in ide_device_put() and in "rmmod at91_ide" function del_gendisk()
cause call to drive_release_dev(), which free drive->id before ide_disk_flush() .
This function oops with NULL driver->id.

There is no oops with my workaround, when I just remove ide_disk_put() from
ide_gd_remove(). It's strange why there is lack of symmetrical _put/_get calls,
ide_gd_probe() has no call to ide_disk_get(). 
  
Cheers
Stanislaw Gruszka

[1]:

[ 5043.790000] Unable to handle kernel NULL pointer dereference at virtual address 000000a6
[ 5043.800000] pgd = c3a40000
[ 5043.800000] [000000a6] *pgd=23b55031, *pte=00000000, *ppte=00000000
[ 5043.810000] Internal error: Oops: 17 [#1]
[ 5043.810000] Modules linked in: ide_gd_mod at91_ide(-) ide_core [last unloaded: ide_gd_mod]
[ 5043.810000] CPU: 0    Not tainted  (2.6.29-rc3 #34)
[ 5043.810000] PC is at ide_disk_flush+0x18/0xe4 [ide_gd_mod]
[ 5043.810000] LR is at ide_gd_remove+0x34/0x40 [ide_gd_mod]
[ 5043.810000] pc : [<bf035c6c>]    lr : [<bf035320>]    psr: 80000013
[ 5043.810000] sp : c3bb1d74  ip : c3bb1db0  fp : c3bb1dac
[ 5043.810000] r10: 0000b8e8  r9 : c3bb0000  r8 : c0028f24
[ 5043.810000] r7 : c3b9cde0  r6 : c3a8de00  r5 : c3b57a00  r4 : c3b98000
[ 5043.810000] r3 : bf037ea0  r2 : 00000000  r1 : 00000051  r0 : c3b98000
[ 5043.810000] Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[ 5043.810000] Control: 0005317f  Table: 23a40000  DAC: 00000015
[ 5043.810000] Process rmmod (pid: 11458, stack limit = 0xc3bb0260)
[ 5043.810000] Stack: (0xc3bb1d74 to 0xc3bb2000)
{snip binary stack}
[ 5043.810000] Backtrace:
[ 5043.810000] [<bf035c54>] (ide_disk_flush+0x0/0xe4 [ide_gd_mod]) from [<bf035320>] (ide_gd_remove+0x34/0x40 [ide_gd_mod])
[ 5043.810000]  r4:c3b98000
[ 5043.810000] [<bf0352ec>] (ide_gd_remove+0x0/0x40 [ide_gd_mod]) from [<bf00b198>] (generic_ide_remove+0x24/0x2c [ide_core])
[ 5043.810000]  r6:c3b984f8 r5:bf03a744 r4:c3b98090
[ 5043.810000] [<bf00b174>] (generic_ide_remove+0x0/0x2c [ide_core]) from [<c0170a38>] (__device_release_driver+0x6c/0x88)
[ 5043.810000] [<c01709cc>] (__device_release_driver+0x0/0x88) from [<c0170b08>] (device_release_driver+0x24/0x30)
[ 5043.810000]  r5:c3b98118 r4:c3b98090
[ 5043.810000] [<c0170ae4>] (device_release_driver+0x0/0x30) from [<c016fd68>] (bus_remove_device+0x80/0x94)
[ 5043.810000]  r5:c3b98090 r4:c3b980c0
[ 5043.810000] [<c016fce8>] (bus_remove_device+0x0/0x94) from [<c016e58c>] (device_del+0x104/0x154)
[ 5043.810000]  r5:c3b98404 r4:c3b98090
[ 5043.810000] [<c016e488>] (device_del+0x0/0x154) from [<c016e5f0>] (device_unregister+0x14/0x20)
[ 5043.810000]  r6:00000001 r5:c3b98404 r4:c3b98090
[ 5043.810000] [<c016e5dc>] (device_unregister+0x0/0x20) from [<bf010614>] (__ide_port_unregister_devices+0x30/0x54 [ide_core])
[ 5043.810000]  r4:c3b98000
[ 5043.810000] [<bf0105e4>] (__ide_port_unregister_devices+0x0/0x54 [ide_core]) from [<bf0106c8>] (ide_host_remove+0x70/0x108 [ide_core])
[ 5043.810000]  r6:00000000 r5:c3bb0000 r4:c3b98400
[ 5043.810000] [<bf010658>] (ide_host_remove+0x0/0x108 [ide_core]) from [<bf023634>] (at91_ide_remove+0x14/0x1c [at91_ide])
[ 5043.810000]  r7:00000880 r6:bf024054 r5:bf024054 r4:c02f3500
[ 5043.810000] [<bf023620>] (at91_ide_remove+0x0/0x1c [at91_ide]) from [<c0171b40>] (platform_drv_remove+0x20/0x24)
[ 5043.810000] [<c0171b20>] (platform_drv_remove+0x0/0x24) from [<c0170a38>] (__device_release_driver+0x6c/0x88)
[ 5043.810000] [<c01709cc>] (__device_release_driver+0x0/0x88) from [<c0170abc>] (driver_detach+0x68/0x90)
[ 5043.810000]  r5:c02f3588 r4:c02f3500
[ 5043.810000] [<c0170a54>] (driver_detach+0x0/0x90) from [<c016fc40>] (bus_remove_driver+0x8c/0xb4)
[ 5043.810000]  r6:c0307320 r5:bf0240a0 r4:bf024054
[ 5043.810000] [<c016fbb4>] (bus_remove_driver+0x0/0xb4) from [<c0170f54>] (driver_unregister+0x44/0x48)
[ 5043.810000]  r6:00000000 r5:bf0240a0 r4:bf024054
[ 5043.810000] [<c0170f10>] (driver_unregister+0x0/0x48) from [<c0171ce0>] (platform_driver_unregister+0x14/0x18)
[ 5043.810000]  r6:bf0241c0 r5:bf0240a0 r4:00000000
[ 5043.810000] [<c0171ccc>] (platform_driver_unregister+0x0/0x18) from [<bf023618>] (at91_ide_exit+0x14/0x1c [at91_ide])
[ 5043.810000] [<bf023604>] (at91_ide_exit+0x0/0x1c [at91_ide]) from [<c005b850>] (sys_delete_module+0x1b8/0x230)
[ 5043.810000] [<c005b698>] (sys_delete_module+0x0/0x230) from [<c0028d80>] (ret_fast_syscall+0x0/0x2c)
[ 5043.810000]  r7:00000081 r6:becd1bcc r5:00000880 r4:becd1cd8
[ 5043.810000] Code: e24cb004 e24dd028 e590201c e1a04000 (e1d21ab6)
[ 5044.320000] ---[ end trace 120de1a999313176 ]---