Re: [PATCH] gfs2: Fix error path kobject memory leak
On Mon, May 13, 2019 at 3:37 PM Andreas Gruenbacher wrote: > > Sorry, I should have been more explicit. Would you mind taking this > patch, please? If it's more convenient or more appropriate, I'll send > a pull request instead. Done. However,I'd like to point out that when I see patches from people who I normally get a pull request from, I usually ignore them. Particularly when they are in some thread with discussion, I'll often just assume that th epatch is part of the thread, not really meant for me in particular. In this case I happened to notice that suddenly my participation status changed, which is why I asked, but in general I might hav ejust archived the thread with the assumption that I'll be getting the patch later as a git pull. Just so you'll be aware of this in the future, in case I don't react... Linus
Re: [PATCH] gfs2: Fix error path kobject memory leak
On Mon, 13 May 2019 at 12:40, Tobin C. Harding wrote: > > On Mon, May 13, 2019 at 09:14:05AM +0200, Greg Kroah-Hartman wrote: > > On Mon, May 13, 2019 at 01:32:13PM +1000, Tobin C. Harding wrote: > > > If a call to kobject_init_and_add() fails we must call kobject_put() > > > otherwise we leak memory. > > > > > > Function always calls kobject_init_and_add() which always calls > > > kobject_init(). > > > > > > It is safe to leave object destruction up to the kobject release > > > function and never free it manually. > > > > > > Remove call to kfree() and always call kobject_put() in the error path. > > > > > > Signed-off-by: Tobin C. Harding > > > --- > > > > > > Is it ok to send patches during the merge window? > > > > > > Applies on top of Linus' mainline tag: v5.1 > > > > > > Happy to rebase if there are conflicts. > > > > > > thanks, > > > Tobin. > > > > > > fs/gfs2/sys.c | 7 +-- > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c > > > index 1787d295834e..98586b139386 100644 > > > --- a/fs/gfs2/sys.c > > > +++ b/fs/gfs2/sys.c > > > @@ -661,8 +661,6 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) > > > if (error) > > > goto fail_reg; > > > > > > - sysfs_frees_sdp = 1; /* Freeing sdp is now done by sysfs calling > > > - function gfs2_sbd_release. */ > > > > You should also delete this variable at the top of the function, as it > > is now only set once there and never used. > > Thanks, I should have gotten a compiler warning for that. I was feeling > so confident with my builds this morning ... pays not to get too cocky > I suppose. > > > With that: > > > > Reviewed-by: Greg Kroah-Hartman > > Thanks, will re-spin. Don't bother, I'll fix that up. Thanks, Andreas
Re: [PATCH] gfs2: Fix error path kobject memory leak
On Mon, May 13, 2019 at 09:14:05AM +0200, Greg Kroah-Hartman wrote: > On Mon, May 13, 2019 at 01:32:13PM +1000, Tobin C. Harding wrote: > > If a call to kobject_init_and_add() fails we must call kobject_put() > > otherwise we leak memory. > > > > Function always calls kobject_init_and_add() which always calls > > kobject_init(). > > > > It is safe to leave object destruction up to the kobject release > > function and never free it manually. > > > > Remove call to kfree() and always call kobject_put() in the error path. > > > > Signed-off-by: Tobin C. Harding > > --- > > > > Is it ok to send patches during the merge window? > > > > Applies on top of Linus' mainline tag: v5.1 > > > > Happy to rebase if there are conflicts. > > > > thanks, > > Tobin. > > > > fs/gfs2/sys.c | 7 +-- > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c > > index 1787d295834e..98586b139386 100644 > > --- a/fs/gfs2/sys.c > > +++ b/fs/gfs2/sys.c > > @@ -661,8 +661,6 @@ int gfs2_sys_fs_add(struct gfs2_sbd *sdp) > > if (error) > > goto fail_reg; > > > > - sysfs_frees_sdp = 1; /* Freeing sdp is now done by sysfs calling > > - function gfs2_sbd_release. */ > > You should also delete this variable at the top of the function, as it > is now only set once there and never used. Thanks, I should have gotten a compiler warning for that. I was feeling so confident with my builds this morning ... pays not to get too cocky I suppose. > With that: > > Reviewed-by: Greg Kroah-Hartman Thanks, will re-spin. Tobin.
