tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

i82875p_edac: fix module remove

Fix module removal bugs of i82875p_edac. Also i82975x_edac code seems to
have the same module removal bugs as in i82875p_edac.

The problems were:

1. In module removal i82875p_remove_one() is never called.

Variable i82875p_registered is newer changed from 1, which
guarantees i82875p_remove_one() is not called (and even if it were
called, it would be called in wrong order).

As a result, the edac_mc workque is not stopped and keeps probing.
If kernel debugging options are not enabled, user may not notice
anything going wrong.

if debugging options are enabled and I do "rmmod i82875p_edac", I
get:

edac debug: edac_pci_workq_function() checking
BUG: unable to handle kernel paging request at f882d16f
...
call trace:
[<f8834df3>] ? edac_mc_workq_function+0x55/0x7e [edac_core]
[<c0233974>] ? run_workqueue+0xd7/0x1a5
[<c023392f>] ? run_workqueue+0x92/0x1a5
[<f8834d9e>] ? edac_mc_workq_function+0x0/0x7e [edac_core]
[<c0233af9>] ? worker_thread+0xb7/0xc3
[<c0236a7b>] ? autoremove_wake_function+0x0/0x33
[<c0233a42>] ? worker_thread+0x0/0xc3
[<c0236809>] ? kthread+0x3b/0x61
[<c02367ce>] ? kthread+0x0/0x61
[<c0204587>] ? kernel_thread_helper+0x7/0x10

Fix for this is to get rid of needles variable i82875p_registered
altogether and run i82875p_remove_one() *before*
pci_unregister_driver().

2. edac_mc_del_mc() uses mci after freeing mci

edac_mc_del_mc() calls calls edac_remove_sysfs_mci_device(). The
kobject refcount of mci drops to 0 and mci is freed. After this
mci is accessed via debug print and i82875p_remove_one() still
uses mci->pvt and tries to free mci again with edac_mc_free().

The fix for this is add kobject_get(&mci->edac_mci_kobj) after
edac_mc_alloc(). Then the mci is still available after returning
from edac_mc_del_mc() with refcount 1, and mci->pvt is still
available. When i82875p_remove_one() finally calls edac_mc_free(),
this will cause kobject_put() and mci is released properly.

Signed-off-by: Jarkko Lavinen <jlavi@iki.fi>
Cc: Doug Thompson <norsk5@yahoo.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Jarkko Lavinen and committed by
Linus Torvalds 09a81269 307d1144

+7 -6
+7 -6
drivers/edac/i82875p_edac.c
··· 182 182 * already registered driver 183 183 */ 184 184 185 - static int i82875p_registered = 1; 186 - 187 185 static struct edac_pci_ctl_info *i82875p_pci; 188 186 189 187 static void i82875p_get_error_info(struct mem_ctl_info *mci, ··· 408 410 goto fail0; 409 411 } 410 412 413 + /* Keeps mci available after edac_mc_del_mc() till edac_mc_free() */ 414 + kobject_get(&mci->edac_mci_kobj); 415 + 411 416 debugf3("%s(): init mci\n", __func__); 412 417 mci->dev = &pdev->dev; 413 418 mci->mtype_cap = MEM_FLAG_DDR; ··· 453 452 return 0; 454 453 455 454 fail1: 455 + kobject_put(&mci->edac_mci_kobj); 456 456 edac_mc_free(mci); 457 457 458 458 fail0: ··· 581 579 { 582 580 debugf3("%s()\n", __func__); 583 581 582 + i82875p_remove_one(mci_pdev); 583 + pci_dev_put(mci_pdev); 584 + 584 585 pci_unregister_driver(&i82875p_driver); 585 586 586 - if (!i82875p_registered) { 587 - i82875p_remove_one(mci_pdev); 588 - pci_dev_put(mci_pdev); 589 - } 590 587 } 591 588 592 589 module_init(i82875p_init);