Geyslan G. Bem
2013-Oct-09 23:40 UTC
[PATCH v3] btrfs: Fix memory leakage in the tree-log.c
In some cases, add_inode_ref() is returning without freeing
the ''name'' pointer.
Added bail out to explicitly call kfree when necessary.
Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
fs/btrfs/tree-log.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 79f057c..ad7cc5f 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1170,13 +1170,16 @@ static noinline int add_inode_ref(struct
btrfs_trans_handle *trans,
if (!dir)
dir = read_one_inode(root, parent_objectid);
if (!dir)
- return -ENOENT;
+ {
+ ret = -ENOENT;
+ goto bail;
+ }
} else {
ret = ref_get_fields(eb, ref_ptr, &namelen, &name,
&ref_index);
}
if (ret)
- return ret;
+ goto bail;
/* if we already have a perfect match, we''re done */
if (!inode_in_dir(root, path, btrfs_ino(dir), btrfs_ino(inode),
@@ -1227,6 +1230,9 @@ out:
btrfs_release_path(path);
iput(dir);
iput(inode);
+bail:
+ if (name)
+ kfree(name);
return ret;
}
--
1.8.4
Geyslan Gregório Bem
2013-Oct-09 23:46 UTC
Re: [PATCH v3] btrfs: Fix memory leakage in the tree-log.c
Please, Analyze [PATCH v3]. Regards, Geyslan Gregório Bem hackingbits.com 2013/10/9 Geyslan G. Bem <geyslan@gmail.com>:> In some cases, add_inode_ref() is returning without freeing > the ''name'' pointer. > > Added bail out to explicitly call kfree when necessary. > > Signed-off-by: Geyslan G. Bem <geyslan@gmail.com> > --- > fs/btrfs/tree-log.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index 79f057c..ad7cc5f 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -1170,13 +1170,16 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, > if (!dir) > dir = read_one_inode(root, parent_objectid); > if (!dir) > - return -ENOENT; > + { > + ret = -ENOENT; > + goto bail; > + } > } else { > ret = ref_get_fields(eb, ref_ptr, &namelen, &name, > &ref_index); > } > if (ret) > - return ret; > + goto bail; > > /* if we already have a perfect match, we''re done */ > if (!inode_in_dir(root, path, btrfs_ino(dir), btrfs_ino(inode), > @@ -1227,6 +1230,9 @@ out: > btrfs_release_path(path); > iput(dir); > iput(inode); > +bail: > + if (name) > + kfree(name); > return ret; > } > > -- > 1.8.4 >
Josef Bacik
2013-Oct-10 00:52 UTC
Re: [PATCH v3] btrfs: Fix memory leakage in the tree-log.c
On Wed, Oct 09, 2013 at 08:40:30PM -0300, Geyslan G. Bem wrote:> In some cases, add_inode_ref() is returning without freeing > the ''name'' pointer. > > Added bail out to explicitly call kfree when necessary. > > Signed-off-by: Geyslan G. Bem <geyslan@gmail.com> > --- > fs/btrfs/tree-log.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index 79f057c..ad7cc5f 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -1170,13 +1170,16 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, > if (!dir) > dir = read_one_inode(root, parent_objectid); > if (!dir) > - return -ENOENT; > + { > + ret = -ENOENT; > + goto bail; > + }Code formatting is if () { } not if () { }> } else { > ret = ref_get_fields(eb, ref_ptr, &namelen, &name, > &ref_index); > } > if (ret) > - return ret; > + goto bail; > > /* if we already have a perfect match, we''re done */ > if (!inode_in_dir(root, path, btrfs_ino(dir), btrfs_ino(inode), > @@ -1227,6 +1230,9 @@ out: > btrfs_release_path(path); > iput(dir); > iput(inode); > +bail: > + if (name) > + kfree(name);kfree already does the if (name) part of this so this is redundant. Also if you are going to do this you need to set name = NULL; after the kfree above it otherwise we have a double free. Thanks, Josef
Geyslan Gregório Bem
2013-Oct-10 01:19 UTC
Re: [PATCH v3] btrfs: Fix memory leakage in the tree-log.c
Josef, Thank you. Sending v4. Geyslan Gregório Bem hackingbits.com 2013/10/9 Josef Bacik <jbacik@fusionio.com>:> On Wed, Oct 09, 2013 at 08:40:30PM -0300, Geyslan G. Bem wrote: >> In some cases, add_inode_ref() is returning without freeing >> the ''name'' pointer. >> >> Added bail out to explicitly call kfree when necessary. >> >> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com> >> --- >> fs/btrfs/tree-log.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c >> index 79f057c..ad7cc5f 100644 >> --- a/fs/btrfs/tree-log.c >> +++ b/fs/btrfs/tree-log.c >> @@ -1170,13 +1170,16 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, >> if (!dir) >> dir = read_one_inode(root, parent_objectid); >> if (!dir) >> - return -ENOENT; >> + { >> + ret = -ENOENT; >> + goto bail; >> + } > > Code formatting is > > if () { > } > > not > > if () > { > } > > >> } else { >> ret = ref_get_fields(eb, ref_ptr, &namelen, &name, >> &ref_index); >> } >> if (ret) >> - return ret; >> + goto bail; >> >> /* if we already have a perfect match, we''re done */ >> if (!inode_in_dir(root, path, btrfs_ino(dir), btrfs_ino(inode), >> @@ -1227,6 +1230,9 @@ out: >> btrfs_release_path(path); >> iput(dir); >> iput(inode); >> +bail: >> + if (name) >> + kfree(name); > > kfree already does the if (name) part of this so this is redundant. Also if you > are going to do this you need to set name = NULL; after the kfree above it > otherwise we have a double free. Thanks, > > Josef