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 <[email protected]>
> ---
> 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
>
>
