<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, [email protected] 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 <[email protected]> > > 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
