Messages in this thread Patch in this message | | | Subject | Re: mfd: lpc_ich: NULL pointer dereference at (second) module removal | From | Peter Tyser <> | Date | Mon, 12 Nov 2012 11:31:15 -0600 |
| |
Thanks for reporting the issue!
On Fri, 2012-11-09 at 14:19 +0100, Paul Bolle wrote: > 0) I can trigger a NULL pointer dereference if I remove the lpc_ich > module. This seems to only happen if I remove it for the second time > (ie, remove the module, insert it and remove it again). This happens > both on i686 and x86_64 (different setups, as inserting the module > triggers different messages about the initialization of the MFD cells on > these machines). Both machines are running v3.6.6.
I believe this is caused by the fact that non-MFD devices get attached to the same parent as the iTCO_wdt driver, which is an MFD. When the MFD code attempts unregister the MFD drivers, it oops when the non-MFD devices are accessed since they don't have the mfd_cell node. I was able to trigger the problem on the first removal of the lpc_ich driver by doing: $ insmod iTCO_wdt.ko $ insmod lpc_ich.ko $ rmmod lpc_ich
> 1) On x86_64 the Oops looks like this: > [...] > <6>[11783.359637] iTCO_wdt: Found a ICH8M-E TCO device (Version=2, TCOBASE=0x1060) > <6>[11783.360477] iTCO_wdt: initialized. heartbeat=30 sec (nowayout=0) > <4>[11783.360492] ACPI Warning: 0x0000000000001028-0x000000000000102f SystemIO conflicts with Region \_SB_.PCI0.LPC_.PMIO 1 (20120711/utaddress-251) > <6>[11783.360498] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver > <4>[11783.360503] ACPI Warning: 0x0000000000001180-0x00000000000011bf SystemIO conflicts with Region \_SB_.PCI0.LPC_.LPIO 1 (20120711/utaddress-251) > <6>[11783.360507] ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver > <4>[11783.360509] lpc_ich: Resource conflict(s) found affecting gpio_ich > [modprobe -r lcp_ich must have been done in these two seconds] > <1>[11785.617128] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
<snip>
> 3) So to me it looks like "cell" is NULL here, and we oops when we're > trying to access "cell->usage_count" (which will then be at offset > 0x10). I have no idea how this can happen.
Your analysis looks correct. This patch should fix the problem I believe.
--- a/drivers/mfd/mfd-core.c +++ b/drivers/mfd/mfd-core.c @@ -208,6 +208,14 @@ static int mfd_remove_devices_fn(struct device *dev, void *c) const struct mfd_cell *cell = mfd_get_cell(pdev); atomic_t **usage_count = c; + /* + * Ignore non-MFD devices. MFD and non-MFD devices can be children of + * the same parent in some cases. Eg the iTCO WDT is a MFD driver, but + * the common WDT core and dev drivers it instantiates are not. + */ + if (!cell) + return 0; + /* find the base address of usage_count pointers (for freeing) */ if (!*usage_count || (cell->usage_count < *usage_count)) *usage_count = cell->usage_count; Samuel, let me know if you have any comments on the fix, otherwise I'll submit a proper patch shortly.
> 4) Side note: why does the kernel print offsets in hex and gdb in > decimal? (Of course, here it's trivial to realize that 0x1d is 29.)
Note sure about that one...
Thanks for the report.
Best, Peter
| |