lkml.org 
[lkml]   [2012]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] 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>
    ---
    drivers/edac/edac_mc.c | 12 +++++++-----
    1 file changed, 7 insertions(+), 5 deletions(-)

    --- linux.orig/drivers/edac/edac_mc.c 2012-08-12 10:10:38.115520521 +0800
    +++ linux/drivers/edac/edac_mc.c 2012-09-23 07:30:40.382206820 +0800
    @@ -419,14 +419,16 @@ error:
    kfree(mci->dimms);
    }
    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(csr);
    }
    - kfree(mci->csrows[i]);
    }
    kfree(mci->csrows);
    }

    \
     
     \ /
      Last update: 2012-09-23 02:41    [W:0.026 / U:30.948 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site