path: root/drivers/edac
diff options
authorFengguang Wu <fengguang.wu@intel.com>2012-09-23 08:18:06 +0800
committerLinus Torvalds <torvalds@linux-foundation.org>2012-09-23 14:45:26 -0700
commitef6e7816b4546475d04b4ea22d58c48472157c70 (patch)
tree6589164940cf4a0f113b1f48dbcce60391112c4e /drivers/edac
parente5e77cf9f9a275083f9a365a20d956fa8a67803e (diff)
edac_mc: fix messy kfree calls in the error path
coccinelle warns about: + drivers/edac/edac_mc.c:429:9-23: ERROR: reference preceded by free on line 429 421 if (mci->csrows) { > 422 for (chn = 0; chn < tot_channels; chn++) { 423 csr = mci->csrows[chn]; 424 if (csr) { > 425 for (chn = 0; chn < tot_channels; chn++) 426 kfree(csr->channels[chn]); 427 kfree(csr); 428 } > 429 kfree(mci->csrows[i]); 430 } 431 kfree(mci->csrows); 432 } and that code block seem to mess things up in several ways (double free, memory leak, out-of-bound reads etc.): L422: The iterator "chn" and bound "tot_channels" are totally wrong. Should be "row" and "tot_csrows" respectively. Which means either memory leak, or out-of-bound reads (which if does not trigger an immediate page fault error, will further lead to kfree() on random addresses). L425: The inner loop is reusing the same iterator "chn" as the outer loop, which could lead to premature end of the outer loop, and hence memory leak. L429: The array index 'i' in mci->csrows[i] is a temporary value used in previous loops, and won't change at all in the current loop. Which means either out-of-bound read and possibly kfree(random number), or the same mci->csrows[i] get freed once and again, and possibly double free for the kfree(csr) in L427. L426/L427: a kfree(csr->channels) is needed in between to avoid leaking the memory. The buggy code was introduced by commit de3910eb ("edac: change the mem allocation scheme to make Documentation/kobject.txt happy") in the 3.6-rc1 merge window. Fix it by freeing up resources in this order: free csrows[i]->channels[j] free csrows[i]->channels free csrows[i] free csrows CC: Mauro Carvalho Chehab <mchehab@redhat.com> CC: Shaun Ruffell <sruffell@digium.com> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'drivers/edac')
1 files changed, 7 insertions, 5 deletions
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 616d90bcb3a4..9037ffa100d5 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -419,14 +419,16 @@ error:
if (mci->csrows) {
- for (chn = 0; chn < tot_channels; chn++) {
- csr = mci->csrows[chn];
+ for (row = 0; row < tot_csrows; row++) {
+ csr = mci->csrows[row];
if (csr) {
- for (chn = 0; chn < tot_channels; chn++)
- kfree(csr->channels[chn]);
+ if (csr->channels) {
+ for (chn = 0; chn < tot_channels; chn++)
+ kfree(csr->channels[chn]);
+ kfree(csr->channels);
+ }
- kfree(mci->csrows[i]);