Messages in this thread Patch in this message | | | Subject | Re: [PATCH] GFS2: Fix mem leak in gfs2_get_acl() | From | Steven Whitehouse <> | Date | Mon, 23 Apr 2012 12:12:55 +0100 |
| |
Hi,
On Sat, 2012-04-21 at 23:00 +0200, Jesper Juhl wrote: > If gfs2_xattr_acl_get() returns 0 - which, as far as I can tell, it > may do independently of having allocated memory for its third argument > ('data' in this case) - then we may leak the memory allocated to data. > I'm not so sure... in gfs2_xattr_acl_get() we have:
error = gfs2_ea_find(ip, GFS2_EATYPE_SYS, name, &el); if (error) return error; if (!el.el_ea) goto out; if (!GFS2_EA_DATA_LEN(el.el_ea)) <---- zero length means return without allocating goto out;
len = GFS2_EA_DATA_LEN(el.el_ea); data = kmalloc(len, GFP_NOFS); etc.
So it looks to me as if we will never have allocated any data unless the length is greater than zero, unless I've missed something?
Steve.
> This patch initializes 'data' to NULL so that it will be safe to call > kfree() on it even if we do not allocate anything and also makes sure > that we kfree(data) in the 'len == 0' case. > > Signed-off-by: Jesper Juhl <jj@chaosbits.net> > --- > fs/gfs2/acl.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > Note: I have tested that this change compiles. It has seen no other > testing than that. > > diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c > index 230eb0f..d254d98 100644 > --- a/fs/gfs2/acl.c > +++ b/fs/gfs2/acl.c > @@ -43,7 +43,7 @@ struct posix_acl *gfs2_get_acl(struct inode *inode, int type) > struct gfs2_inode *ip = GFS2_I(inode); > struct posix_acl *acl; > const char *name; > - char *data; > + char *data = NULL; > int len; > > if (!ip->i_eattr) > @@ -60,8 +60,10 @@ struct posix_acl *gfs2_get_acl(struct inode *inode, int type) > len = gfs2_xattr_acl_get(ip, name, &data); > if (len < 0) > return ERR_PTR(len); > - if (len == 0) > + if (len == 0) { > + kfree(data); > return NULL; > + } > > acl = posix_acl_from_xattr(data, len); > kfree(data); > -- > 1.7.10 > >
| |