All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shaun Ruffell <sruffell@digium.com>
To: Mauro Carvalho Chehab <mchehab@redhat.com>,
	Fengguang Wu <fengguang.wu@intel.com>
Cc: linux-kernel@vger.kernel.org, linux-edac@vger.kernel.org
Subject: [PATCH 2/3] edac: edac_mc_free() cannot assume mem_ctl_info is registered in sysfs.
Date: Sat, 18 Aug 2012 23:11:23 -0500	[thread overview]
Message-ID: <1345349484-31552-3-git-send-email-sruffell@digium.com> (raw)
In-Reply-To: <1345349484-31552-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..a58facc 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 (!mci->bus.name) {
+		_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-08-19  4:47 UTC|newest]

Thread overview: 14+ 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-10  9:22     ` 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       ` Shaun Ruffell [this message]
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         ` [PATCH v2 2/3] edac: edac_mc_free() cannot assume mem_ctl_info is registered in sysfs Shaun Ruffell
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=1345349484-31552-3-git-send-email-sruffell@digium.com \
    --to=sruffell@digium.com \
    --cc=fengguang.wu@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.