lkml.org 
[lkml]   [2012]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH][RFC] XFS: Fix mem leak and possible NULL deref in xfs_setattr_nonsize()
    In xfs_setattr_nonsize(), xfs_trans_alloc() gets its memory from
    _xfs_trans_alloc() which gets it from kmem_zone_zalloc() which may
    fail and return NULL. So this:

    tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);

    may result in a NULL 'tp'.
    If it does, then the call:

    error = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp), 0, 0, 0);

    with a NULL 'tp' will explode, since xfs_trans_reserve() dereferences
    its first argument unconditionally.

    And if the memory allocation for 'tp' goes well (and thus
    xfs_trans_reserve() does not explode) then we may leak the memory
    allocated to 'tp' if xfs_trans_reserve() returns error.

    I believe this patch should fix both issues, but I'm not intimate with
    the XFS code at all, so there can easily be something I overlooked or
    something that should be done differently than what I did.

    Signed-off-by: Jesper Juhl <jj@chaosbits.net>
    ---
    fs/xfs/xfs_iops.c | 7 ++++++-
    1 files changed, 6 insertions(+), 1 deletions(-)

    Note:
    Please review carefully before applying.
    Especially since I don't currently have any XFS filesystems to test
    this on, nor any clear idea of a good way to actually test this if I
    had. So this patch is compile tested only on my end.

    diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
    index ab30253..194c9d7 100644
    --- a/fs/xfs/xfs_iops.c
    +++ b/fs/xfs/xfs_iops.c
    @@ -575,9 +575,14 @@ xfs_setattr_nonsize(
    }

    tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_NOT_SIZE);
    + if (!tp)
    + goto out_dqrele;
    +
    error = xfs_trans_reserve(tp, 0, XFS_ICHANGE_LOG_RES(mp), 0, 0, 0);
    - if (error)
    + if (error) {
    + xfs_trans_cancel(tp, 0);
    goto out_dqrele;
    + }

    xfs_ilock(ip, XFS_ILOCK_EXCL);

    --
    1.7.9

    Please CC me on replies.

    --
    Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
    Don't top-post http://www.catb.org/jargon/html/T/top-post.html
    Plain text mails only, please.



    \
     
     \ /
      Last update: 2012-02-05 22:25    [W:0.035 / U:31.772 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site