[PATCH] ext4: Fix up bh refcount in xattr code.

From: Dave Jones
Date: Wed Oct 30 2013 - 14:33:53 EST


<post kernel-summit re-ping>

On Tue, Oct 15, 2013 at 02:11:42PM -0400, Dave Jones wrote:
> On Mon, Oct 14, 2013 at 01:31:23PM -0700, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
>
> > If we take the 2nd retry path in ext4_expand_extra_isize_ea, we
> > potentionally return from the function without having freed these
> > allocations. If we don't do the return, we over-write the previous
> > allocation pointers, so we leak either way.
> >
> > Spotted with Coverity.
> >
> > [ Fixed by tytso to set is and bs to NULL after freeing these
> > pointers, in case in the retry loop we later end up triggering an
> > error causing a jump to cleanup, at which point we could have a double
> > free bug. -- Ted ]
> >
> > ---
> > fs/ext4/xattr.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -1271,6 +1271,8 @@ retry:
> > s_min_extra_isize) {
> > tried_min_extra_isize++;
> > new_extra_isize = s_min_extra_isize;
> > + kfree(is); is = NULL;
> > + kfree(bs); bs = NULL;
> > goto retry;
> > }
>
> Because this was different in my local tree I ended up looking it over again
> when I rebased. Do we also need the patch below ?
>
> Dave
>
> If we take the retry path here, we end up potentially overwriting bh, leaving
> it with an elevated reference count.
>
> Signed-off-by: Dave Jones <davej@xxxxxxxxxxxxxxxxx>
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 03e9beb..1423c48 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1352,6 +1352,7 @@ retry:
> new_extra_isize = s_min_extra_isize;
> kfree(is); is = NULL;
> kfree(bs); bs = NULL;
> + brelse(bh);
> goto retry;
> }
> error = -1;
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/