2010-12-15 21:57:53

by Mariusz Kozlowski

[permalink] [raw]
Subject: [PATCH] btrfs: fix memory leak on error path in btrfs_get_acl()

If posix_acl_from_xattr() fails we leak memory stored in 'value'.

Signed-off-by: Mariusz Kozlowski <[email protected]>
---
fs/btrfs/acl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 2222d16..11c9561 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -61,6 +61,7 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
if (size > 0) {
acl = posix_acl_from_xattr(value, size);
if (IS_ERR(acl))
+ kfree(value);
return acl;
set_cached_acl(inode, type, acl);
}
--
1.7.0.4


2010-12-15 22:49:42

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] btrfs: fix memory leak on error path in btrfs_get_acl()

On Wed, Dec 15, 2010 at 10:57 PM, Mariusz Kozlowski <[email protected]> wrote:
> If posix_acl_from_xattr() fails we leak memory stored in 'value'.
>
> Signed-off-by: Mariusz Kozlowski <[email protected]>
> ---
> ?fs/btrfs/acl.c | ? ?1 +
> ?1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 2222d16..11c9561 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -61,6 +61,7 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
> ? ? ? ? ? ? ? ?if (size > 0) {
> ? ? ? ? ? ? ? ? ? ? ? ?acl = posix_acl_from_xattr(value, size);
> ? ? ? ? ? ? ? ? ? ? ? ?if (IS_ERR(acl))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? kfree(value);

Be careful with the evil { }

> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?return acl;
> ? ? ? ? ? ? ? ? ? ? ? ?set_cached_acl(inode, type, acl);
> ? ? ? ? ? ? ? ?}
> --
> 1.7.0.4
>
> --
> 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/
>

2010-12-16 05:56:44

by Mariusz Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] btrfs: fix memory leak on error path in btrfs_get_acl()

On Wed, Dec 15, 2010 at 11:49:39PM +0100, Miguel Ojeda wrote:
> On Wed, Dec 15, 2010 at 10:57 PM, Mariusz Kozlowski <[email protected]> wrote:
> > If posix_acl_from_xattr() fails we leak memory stored in 'value'.
> >
> > Signed-off-by: Mariusz Kozlowski <[email protected]>
> > ---
> > ?fs/btrfs/acl.c | ? ?1 +
> > ?1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> > index 2222d16..11c9561 100644
> > --- a/fs/btrfs/acl.c
> > +++ b/fs/btrfs/acl.c
> > @@ -61,6 +61,7 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
> > ? ? ? ? ? ? ? ?if (size > 0) {
> > ? ? ? ? ? ? ? ? ? ? ? ?acl = posix_acl_from_xattr(value, size);
> > ? ? ? ? ? ? ? ? ? ? ? ?if (IS_ERR(acl))
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? kfree(value);
>
> Be careful with the evil { }

Dang. Too much python recently i guess. Will send v2 shortly.

Thanks.

> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?return acl;
> > ? ? ? ? ? ? ? ? ? ? ? ?set_cached_acl(inode, type, acl);
> > ? ? ? ? ? ? ? ?}
> > --
> > 1.7.0.4
> >
> > --
> > 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/
> >

--
Mariusz Kozlowski

2010-12-16 06:10:29

by Mariusz Kozlowski

[permalink] [raw]
Subject: [PATCH v2] btrfs: fix memory leak on error path in btrfs_get_acl()

If posix_acl_from_xattr() fails we leak memory stored in 'value'.

Signed-off-by: Mariusz Kozlowski <[email protected]>
---
fs/btrfs/acl.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 2222d16..6d1410e 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -60,8 +60,10 @@ static struct posix_acl *btrfs_get_acl(struct inode *inode, int type)
size = __btrfs_getxattr(inode, name, value, size);
if (size > 0) {
acl = posix_acl_from_xattr(value, size);
- if (IS_ERR(acl))
+ if (IS_ERR(acl)) {
+ kfree(value);
return acl;
+ }
set_cached_acl(inode, type, acl);
}
kfree(value);
--
1.7.0.4