Skip to content

Commit faa2ad0

Browse files
sruffelltorvalds
authored andcommitted
edac_mc: edac_mc_free() cannot assume mem_ctl_info is registered in sysfs.
Fix potential NULL pointer dereference in edac_unregister_sysfs() on system boot introduced in 3.6-rc1. Since commit 7a623c0 ("edac: rewrite the sysfs code to use struct device") edac_mc_alloc() no longer initializes embedded kobjects in struct mem_ctl_info. Therefore edac_mc_free() can no longer simply decrement a kobject reference count to free the allocated memory unless the memory controller driver module had also called edac_mc_add_mc(). Now edac_mc_free() will check if the newly embedded struct device has been registered with sysfs before using either the standard device release functions or freeing the data structures itself with logic pulled out of the error path of edac_mc_alloc(). The BUG this patch resolves for me: BUG: unable to handle kernel NULL pointer dereference at (null) EIP is at __wake_up_common+0x1a/0x6a Process modprobe (pid: 933, ti=f3dc6000 task=f3db9520 task.ti=f3dc6000) Call Trace: complete_all+0x3f/0x50 device_pm_remove+0x23/0xa2 device_del+0x34/0x142 edac_unregister_sysfs+0x3b/0x5c [edac_core] edac_mc_free+0x29/0x2f [edac_core] e7xxx_probe1+0x268/0x311 [e7xxx_edac] e7xxx_init_one+0x56/0x61 [e7xxx_edac] local_pci_probe+0x13/0x15 ... Cc: Mauro Carvalho Chehab <[email protected]> Cc: Shaohui Xie <[email protected]> Signed-off-by: Shaun Ruffell <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent ef6e781 commit faa2ad0

File tree

1 file changed

+39
-20
lines changed

1 file changed

+39
-20
lines changed

drivers/edac/edac_mc.c

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,36 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
199199
return (void *)(((unsigned long)ptr) + align - r);
200200
}
201201

202+
static void _edac_mc_free(struct mem_ctl_info *mci)
203+
{
204+
int i, chn, row;
205+
struct csrow_info *csr;
206+
const unsigned int tot_dimms = mci->tot_dimms;
207+
const unsigned int tot_channels = mci->num_cschannel;
208+
const unsigned int tot_csrows = mci->nr_csrows;
209+
210+
if (mci->dimms) {
211+
for (i = 0; i < tot_dimms; i++)
212+
kfree(mci->dimms[i]);
213+
kfree(mci->dimms);
214+
}
215+
if (mci->csrows) {
216+
for (row = 0; row < tot_csrows; row++) {
217+
csr = mci->csrows[row];
218+
if (csr) {
219+
if (csr->channels) {
220+
for (chn = 0; chn < tot_channels; chn++)
221+
kfree(csr->channels[chn]);
222+
kfree(csr->channels);
223+
}
224+
kfree(csr);
225+
}
226+
}
227+
kfree(mci->csrows);
228+
}
229+
kfree(mci);
230+
}
231+
202232
/**
203233
* edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
204234
* @mc_num: Memory controller number
@@ -413,26 +443,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
413443
return mci;
414444

415445
error:
416-
if (mci->dimms) {
417-
for (i = 0; i < tot_dimms; i++)
418-
kfree(mci->dimms[i]);
419-
kfree(mci->dimms);
420-
}
421-
if (mci->csrows) {
422-
for (row = 0; row < tot_csrows; row++) {
423-
csr = mci->csrows[row];
424-
if (csr) {
425-
if (csr->channels) {
426-
for (chn = 0; chn < tot_channels; chn++)
427-
kfree(csr->channels[chn]);
428-
kfree(csr->channels);
429-
}
430-
kfree(csr);
431-
}
432-
}
433-
kfree(mci->csrows);
434-
}
435-
kfree(mci);
446+
_edac_mc_free(mci);
436447

437448
return NULL;
438449
}
@@ -447,6 +458,14 @@ void edac_mc_free(struct mem_ctl_info *mci)
447458
{
448459
edac_dbg(1, "\n");
449460

461+
/* If we're not yet registered with sysfs free only what was allocated
462+
* in edac_mc_alloc().
463+
*/
464+
if (!device_is_registered(&mci->dev)) {
465+
_edac_mc_free(mci);
466+
return;
467+
}
468+
450469
/* the mci instance is freed here, when the sysfs object is dropped */
451470
edac_unregister_sysfs(mci);
452471
}

0 commit comments

Comments
 (0)