linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shaun Ruffell <sruffell@digium.com>
To: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org
Subject: [PATCH v2 2/3] edac: edac_mc_free() cannot assume mem_ctl_info is registered in sysfs.
Date: Fri, 14 Sep 2012 12:58:17 -0500	[thread overview]
Message-ID: <1347645498-21215-3-git-send-email-sruffell@digium.com> (raw)
In-Reply-To: <1347645498-21215-1-git-send-email-sruffell@digium.com>

edac_mc_free() may need to deallocate any memory associated with struct
mem_ctl_info directly if the structure was never registered with sysfs in
edac_mc_add_mc(). This moves the error handling code from edac_mc_alloc() into a
dedicated function to be called by edac_mc_free() as well if necessary.

This resolves a NULL pointer dereference from the following code path first
introduced in 3.6-rc1:

  EDAC MC: Ver: 3.0.0
  EDAC DEBUG: edac_mc_sysfs_init: device mc created
  EDAC DEBUG: e7xxx_init_one:
  EDAC DEBUG: e7xxx_probe1: mci
  EDAC DEBUG: edac_mc_alloc: errcount layer 0 size 8
  EDAC DEBUG: edac_mc_alloc: errcount layer 1 size 16
  EDAC DEBUG: edac_mc_alloc: allocating 48 error counters
  EDAC DEBUG: edac_mc_alloc: allocating 1068 bytes for mci data (16 ranks, 16 csrows/channels)
  EDAC DEBUG: e7xxx_probe1: init mci
  EDAC DEBUG: e7xxx_probe1: init pvt
  EDAC e7xxx: error reporting device not found:vendor 8086 device 0x2541 (broken BIOS?)
  EDAC DEBUG: edac_mc_free:
  Floppy drive(s): fd0 is 1.44M
  EDAC DEBUG: edac_unregister_sysfs: Unregistering device (null)

Signed-off-by: Shaun Ruffell <sruffell@digium.com>
---
 drivers/edac/edac_mc.c | 59 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 9037ffa..d5dc9da 100644
--- 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);
 }
-- 
1.7.11.2


  parent reply	other threads:[~2012-09-14 18:05 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-09 13:54 Fengguang Wu
2012-08-09 17:29 ` Mauro Carvalho Chehab
2012-08-10  9:22   ` [PATCH] edac_mc: fix kfree calls in the error path Fengguang Wu
2012-08-19  4:11     ` [PATCH 0/3] Fix edac_mc crash in e7xxx_edac " Shaun Ruffell
2012-08-19  4:11       ` [PATCH 1/3] edac_mc: fix kfree calls in the " Shaun Ruffell
2012-08-19  4:11       ` [PATCH 2/3] edac: edac_mc_free() cannot assume mem_ctl_info is registered in sysfs Shaun Ruffell
2012-08-19  4:11       ` [PATCH 3/3] edac: edac_mc no longer deals with kobjects directly Shaun Ruffell
2012-09-08 18:49       ` [PATCH 0/3] Fix edac_mc crash in e7xxx_edac error path Shaun Ruffell
2012-09-14 17:58       ` [PATCH v2 " Shaun Ruffell
2012-09-14 17:58         ` [PATCH v2 1/3] edac_mc: fix kfree calls in the " Shaun Ruffell
2012-09-14 18:01           ` Shaun Ruffell
2012-09-14 17:58         ` Shaun Ruffell [this message]
2012-09-14 17:58         ` [PATCH v2 3/3] edac: edac_mc no longer deals with kobjects directly Shaun Ruffell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1347645498-21215-3-git-send-email-sruffell@digium.com \
    --to=sruffell@digium.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).