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 : > 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 >> --- >> 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, , , >>_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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 > --- > 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, , , >_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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 : > 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 > --- > 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, , , > _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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[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 --- 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, , , _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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[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 geys...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 geys...@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 geys...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 geys...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 jba...@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 geys...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/