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 7a623c039 ("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 <mchehab@redhat.com>
Cc: Shaohui Xie <Shaohui.Xie@freescale.com>
Signed-off-by: Shaun Ruffell <sruffell@digium.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 9037ffa100d5991abd2323f052590e44dd8f8145..d5dc9da7f99f95fc9dd3a6edb5d4bc302e69c2c3 100644 (file)
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -199,6 +199,36 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
        return (void *)(((unsigned long)ptr) + align - r);
 }
 
+static void _edac_mc_free(struct mem_ctl_info *mci)
+{
+       int i, chn, row;
+       struct csrow_info *csr;
+       const unsigned int tot_dimms = mci->tot_dimms;
+       const unsigned int tot_channels = mci->num_cschannel;
+       const unsigned int tot_csrows = mci->nr_csrows;
+
+       if (mci->dimms) {
+               for (i = 0; i < tot_dimms; i++)
+                       kfree(mci->dimms[i]);
+               kfree(mci->dimms);
+       }
+       if (mci->csrows) {
+               for (row = 0; row < tot_csrows; row++) {
+                       csr = mci->csrows[row];
+                       if (csr) {
+                               if (csr->channels) {
+                                       for (chn = 0; chn < tot_channels; chn++)
+                                               kfree(csr->channels[chn]);
+                                       kfree(csr->channels);
+                               }
+                               kfree(csr);
+                       }
+               }
+               kfree(mci->csrows);
+       }
+       kfree(mci);
+}
+
 /**
  * edac_mc_alloc: Allocate and partially fill a struct mem_ctl_info structure
  * @mc_num:            Memory controller number
@@ -413,26 +443,7 @@ struct mem_ctl_info *edac_mc_alloc(unsigned mc_num,
        return mci;
 
 error:
-       if (mci->dimms) {
-               for (i = 0; i < tot_dimms; i++)
-                       kfree(mci->dimms[i]);
-               kfree(mci->dimms);
-       }
-       if (mci->csrows) {
-               for (row = 0; row < tot_csrows; row++) {
-                       csr = mci->csrows[row];
-                       if (csr) {
-                               if (csr->channels) {
-                                       for (chn = 0; chn < tot_channels; chn++)
-                                               kfree(csr->channels[chn]);
-                                       kfree(csr->channels);
-                               }
-                               kfree(csr);
-                       }
-               }
-               kfree(mci->csrows);
-       }
-       kfree(mci);
+       _edac_mc_free(mci);
 
        return NULL;
 }
@@ -447,6 +458,14 @@ void edac_mc_free(struct mem_ctl_info *mci)
 {
        edac_dbg(1, "\n");
 
+       /* If we're not yet registered with sysfs free only what was allocated
+        * in edac_mc_alloc().
+        */
+       if (!device_is_registered(&mci->dev)) {
+               _edac_mc_free(mci);
+               return;
+       }
+
        /* the mci instance is freed here, when the sysfs object is dropped */
        edac_unregister_sysfs(mci);
 }