LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] bridge: Fix error path for kobject_init_and_add()
@ 2019-05-10  2:52 Tobin C. Harding
  2019-05-10  2:57 ` Tobin C. Harding
  2019-05-10 22:06 ` David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Tobin C. Harding @ 2019-05-10  2:52 UTC (permalink / raw)
  To: David S. Miller
  Cc: Tobin C. Harding, Greg Kroah-Hartman, Roopa Prabhu,
	Nikolay Aleksandrov, Tyler Hicks, bridge, netdev, linux-kernel

Currently error return from kobject_init_and_add() is not followed by a
call to kobject_put().  This means there is a memory leak.  We currently
set p to NULL so that kfree() may be called on it as a noop, the code is
arguably clearer if we move the kfree() up closer to where it is
called (instead of after goto jump).

Remove a goto label 'err1' and jump to call to kobject_put() in error
return from kobject_init_and_add() fixing the memory leak.  Re-name goto
label 'put_back' to 'err1' now that we don't use err1, following current
nomenclature (err1, err2 ...).  Move call to kfree out of the error
code at bottom of function up to closer to where memory was allocated.
Add comment to clarify call to kfree().

Signed-off-by: Tobin C. Harding <tobin@kernel.org>
---

v1 was a part of a set.  I have dropped the other patch until I can work
out a correct solution.

 net/bridge/br_if.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 41f0a696a65f..0cb0aa0313a8 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -602,13 +602,15 @@ int br_add_if(struct net_bridge *br, struct net_device *dev,
 	call_netdevice_notifiers(NETDEV_JOIN, dev);
 
 	err = dev_set_allmulti(dev, 1);
-	if (err)
-		goto put_back;
+	if (err) {
+		kfree(p);	/* kobject not yet init'd, manually free */
+		goto err1;
+	}
 
 	err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
 				   SYSFS_BRIDGE_PORT_ATTR);
 	if (err)
-		goto err1;
+		goto err2;
 
 	err = br_sysfs_addif(p);
 	if (err)
@@ -700,12 +702,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev,
 	sysfs_remove_link(br->ifobj, p->dev->name);
 err2:
 	kobject_put(&p->kobj);
-	p = NULL; /* kobject_put frees */
-err1:
 	dev_set_allmulti(dev, -1);
-put_back:
+err1:
 	dev_put(dev);
-	kfree(p);
 	return err;
 }
 
-- 
2.21.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] bridge: Fix error path for kobject_init_and_add()
  2019-05-10  2:52 [PATCH v2] bridge: Fix error path for kobject_init_and_add() Tobin C. Harding
@ 2019-05-10  2:57 ` Tobin C. Harding
  2019-05-10 22:06 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: Tobin C. Harding @ 2019-05-10  2:57 UTC (permalink / raw)
  To: Tobin C. Harding
  Cc: David S. Miller, Greg Kroah-Hartman, Roopa Prabhu,
	Nikolay Aleksandrov, Tyler Hicks, bridge, netdev, linux-kernel

On Fri, May 10, 2019 at 12:52:12PM +1000, Tobin C. Harding wrote:

Please ignore - I forgot about netdev procedure and the merge window.
My bad.

Will re-send when you are open.

thanks,
Tobin.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] bridge: Fix error path for kobject_init_and_add()
  2019-05-10  2:52 [PATCH v2] bridge: Fix error path for kobject_init_and_add() Tobin C. Harding
  2019-05-10  2:57 ` Tobin C. Harding
@ 2019-05-10 22:06 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2019-05-10 22:06 UTC (permalink / raw)
  To: tobin; +Cc: gregkh, roopa, nikolay, tyhicks, bridge, netdev, linux-kernel

From: "Tobin C. Harding" <tobin@kernel.org>
Date: Fri, 10 May 2019 12:52:12 +1000

> Currently error return from kobject_init_and_add() is not followed by a
> call to kobject_put().  This means there is a memory leak.  We currently
> set p to NULL so that kfree() may be called on it as a noop, the code is
> arguably clearer if we move the kfree() up closer to where it is
> called (instead of after goto jump).
> 
> Remove a goto label 'err1' and jump to call to kobject_put() in error
> return from kobject_init_and_add() fixing the memory leak.  Re-name goto
> label 'put_back' to 'err1' now that we don't use err1, following current
> nomenclature (err1, err2 ...).  Move call to kfree out of the error
> code at bottom of function up to closer to where memory was allocated.
> Add comment to clarify call to kfree().
> 
> Signed-off-by: Tobin C. Harding <tobin@kernel.org>
> ---
> 
> v1 was a part of a set.  I have dropped the other patch until I can work
> out a correct solution.

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-05-10 22:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10  2:52 [PATCH v2] bridge: Fix error path for kobject_init_and_add() Tobin C. Harding
2019-05-10  2:57 ` Tobin C. Harding
2019-05-10 22:06 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).