gfs2: Fix error path kobject memory leak
diff mbox series

Message ID 20190513195904.15726-1-agruenba@redhat.com
State Accepted
Commit fbcde197e1befae4228715edf1288c7646808b8b
Headers show
Series
  • gfs2: Fix error path kobject memory leak
Related show

Commit Message

Andreas Gruenbacher May 13, 2019, 7:59 p.m. UTC
From: "Tobin C. Harding" <tobin@kernel.org>

If a call to kobject_init_and_add() fails we must call kobject_put()
otherwise we leak memory.

Function gfs2_sys_fs_add 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 <tobin@kernel.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/sys.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Linus Torvalds May 13, 2019, 10:27 p.m. UTC | #1
Andreas,
 did you intend for me to take this as a patch from the email, or will
I get a pull request with this later?

             Linus
Andreas Gruenbacher May 13, 2019, 10:37 p.m. UTC | #2
On Tue, 14 May 2019 at 00:27, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Andreas,
>  did you intend for me to take this as a patch from the email, or will
> I get a pull request with this later?

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.

Thanks,
Andreas
Linus Torvalds May 13, 2019, 10:47 p.m. UTC | #3
On Mon, May 13, 2019 at 3:37 PM Andreas Gruenbacher <agruenba@redhat.com> 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
Andreas Gruenbacher May 13, 2019, 10:57 p.m. UTC | #4
On Tue, 14 May 2019 at 00:47, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, May 13, 2019 at 3:37 PM Andreas Gruenbacher <agruenba@redhat.com> 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  the patch 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 have just
> 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...

Thanks.

Copying Jon because this looks like useful information for other
maintainers as well.

Andreas

Patch
diff mbox series

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index 1787d295834e..08e4996adc23 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -650,7 +650,6 @@  int gfs2_sys_fs_add(struct gfs2_sbd *sdp)
 	char ro[20];
 	char spectator[20];
 	char *envp[] = { ro, spectator, NULL };
-	int sysfs_frees_sdp = 0;
 
 	sprintf(ro, "RDONLY=%d", sb_rdonly(sb));
 	sprintf(spectator, "SPECTATOR=%d", sdp->sd_args.ar_spectator ? 1 : 0);
@@ -661,8 +660,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. */
 	error = sysfs_create_group(&sdp->sd_kobj, &tune_group);
 	if (error)
 		goto fail_reg;
@@ -687,10 +684,7 @@  int gfs2_sys_fs_add(struct gfs2_sbd *sdp)
 fail_reg:
 	free_percpu(sdp->sd_lkstats);
 	fs_err(sdp, "error %d adding sysfs files\n", error);
-	if (sysfs_frees_sdp)
-		kobject_put(&sdp->sd_kobj);
-	else
-		kfree(sdp);
+	kobject_put(&sdp->sd_kobj);
 	sb->s_fs_info = NULL;
 	return error;
 }