linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] ethtool: fix reference leak in some *_SET handlers
@ 2020-03-22 20:15 Michal Kubecek
  2020-03-22 20:23 ` Andrew Lunn
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Michal Kubecek @ 2020-03-22 20:15 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev; +Cc: Andrew Lunn, linux-kernel

Andrew noticed that some handlers for *_SET commands leak a netdev
reference if required ethtool_ops callbacks do not exist. A simple
reproducer would be e.g.

  ip link add veth1 type veth peer name veth2
  ethtool -s veth1 wol g
  ip link del veth1

Make sure dev_put() is called when ethtool_ops check fails.

Reported-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 net/ethtool/debug.c     | 4 +++-
 net/ethtool/linkinfo.c  | 4 +++-
 net/ethtool/linkmodes.c | 4 +++-
 net/ethtool/wol.c       | 4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/ethtool/debug.c b/net/ethtool/debug.c
index aaef4843e6ba..92599ad7b3c2 100644
--- a/net/ethtool/debug.c
+++ b/net/ethtool/debug.c
@@ -107,8 +107,9 @@ int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 	dev = req_info.dev;
+	ret = -EOPNOTSUPP;
 	if (!dev->ethtool_ops->get_msglevel || !dev->ethtool_ops->set_msglevel)
-		return -EOPNOTSUPP;
+		goto out_dev;
 
 	rtnl_lock();
 	ret = ethnl_ops_begin(dev);
@@ -129,6 +130,7 @@ int ethnl_set_debug(struct sk_buff *skb, struct genl_info *info)
 	ethnl_ops_complete(dev);
 out_rtnl:
 	rtnl_unlock();
+out_dev:
 	dev_put(dev);
 	return ret;
 }
diff --git a/net/ethtool/linkinfo.c b/net/ethtool/linkinfo.c
index 5d16cb4e8693..6e9e0b590bb5 100644
--- a/net/ethtool/linkinfo.c
+++ b/net/ethtool/linkinfo.c
@@ -126,9 +126,10 @@ int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 	dev = req_info.dev;
+	ret = -EOPNOTSUPP;
 	if (!dev->ethtool_ops->get_link_ksettings ||
 	    !dev->ethtool_ops->set_link_ksettings)
-		return -EOPNOTSUPP;
+		goto out_dev;
 
 	rtnl_lock();
 	ret = ethnl_ops_begin(dev);
@@ -162,6 +163,7 @@ int ethnl_set_linkinfo(struct sk_buff *skb, struct genl_info *info)
 	ethnl_ops_complete(dev);
 out_rtnl:
 	rtnl_unlock();
+out_dev:
 	dev_put(dev);
 	return ret;
 }
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index 96f20be64553..18cc37be2d9c 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -338,9 +338,10 @@ int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 	dev = req_info.dev;
+	ret = -EOPNOTSUPP;
 	if (!dev->ethtool_ops->get_link_ksettings ||
 	    !dev->ethtool_ops->set_link_ksettings)
-		return -EOPNOTSUPP;
+		goto out_dev;
 
 	rtnl_lock();
 	ret = ethnl_ops_begin(dev);
@@ -370,6 +371,7 @@ int ethnl_set_linkmodes(struct sk_buff *skb, struct genl_info *info)
 	ethnl_ops_complete(dev);
 out_rtnl:
 	rtnl_unlock();
+out_dev:
 	dev_put(dev);
 	return ret;
 }
diff --git a/net/ethtool/wol.c b/net/ethtool/wol.c
index e1b8a65b64c4..55e1ecaaf739 100644
--- a/net/ethtool/wol.c
+++ b/net/ethtool/wol.c
@@ -128,8 +128,9 @@ int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info)
 	if (ret < 0)
 		return ret;
 	dev = req_info.dev;
+	ret = -EOPNOTSUPP;
 	if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
-		return -EOPNOTSUPP;
+		goto out_dev;
 
 	rtnl_lock();
 	ret = ethnl_ops_begin(dev);
@@ -172,6 +173,7 @@ int ethnl_set_wol(struct sk_buff *skb, struct genl_info *info)
 	ethnl_ops_complete(dev);
 out_rtnl:
 	rtnl_unlock();
+out_dev:
 	dev_put(dev);
 	return ret;
 }
-- 
2.25.1


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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 20:15 [PATCH net] ethtool: fix reference leak in some *_SET handlers Michal Kubecek
@ 2020-03-22 20:23 ` Andrew Lunn
  2020-03-22 20:42 ` Florian Fainelli
  2020-03-22 20:43 ` Jakub Kicinski
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Lunn @ 2020-03-22 20:23 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Sun, Mar 22, 2020 at 09:15:51PM +0100, Michal Kubecek wrote:
> Andrew noticed that some handlers for *_SET commands leak a netdev
> reference if required ethtool_ops callbacks do not exist. A simple
> reproducer would be e.g.
> 
>   ip link add veth1 type veth peer name veth2
>   ethtool -s veth1 wol g
>   ip link del veth1
> 
> Make sure dev_put() is called when ethtool_ops check fails.
> 
> Reported-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 20:15 [PATCH net] ethtool: fix reference leak in some *_SET handlers Michal Kubecek
  2020-03-22 20:23 ` Andrew Lunn
@ 2020-03-22 20:42 ` Florian Fainelli
  2020-03-22 20:43 ` Jakub Kicinski
  2 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2020-03-22 20:42 UTC (permalink / raw)
  To: Michal Kubecek, David S. Miller, Jakub Kicinski, netdev
  Cc: Andrew Lunn, linux-kernel



On 3/22/2020 1:15 PM, Michal Kubecek wrote:
> Andrew noticed that some handlers for *_SET commands leak a netdev
> reference if required ethtool_ops callbacks do not exist. A simple
> reproducer would be e.g.
> 
>   ip link add veth1 type veth peer name veth2
>   ethtool -s veth1 wol g
>   ip link del veth1
> 
> Make sure dev_put() is called when ethtool_ops check fails.
> 
> Reported-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Would not you want a Fixes: tag for this change?

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 20:15 [PATCH net] ethtool: fix reference leak in some *_SET handlers Michal Kubecek
  2020-03-22 20:23 ` Andrew Lunn
  2020-03-22 20:42 ` Florian Fainelli
@ 2020-03-22 20:43 ` Jakub Kicinski
  2020-03-22 20:51   ` Michal Kubecek
  2 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-03-22 20:43 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: David S. Miller, netdev, Andrew Lunn, linux-kernel

On Sun, 22 Mar 2020 21:15:51 +0100 (CET) Michal Kubecek wrote:
> Andrew noticed that some handlers for *_SET commands leak a netdev
> reference if required ethtool_ops callbacks do not exist. A simple
> reproducer would be e.g.
> 
>   ip link add veth1 type veth peer name veth2
>   ethtool -s veth1 wol g
>   ip link del veth1
> 
> Make sure dev_put() is called when ethtool_ops check fails.

Fixes: e54d04e3afea ("ethtool: set message mask with DEBUG_SET request")
Fixes: a53f3d41e4d3 ("ethtool: set link settings with LINKINFO_SET request")
Fixes: bfbcfe2032e7 ("ethtool: set link modes related data with LINKMODES_SET request")
Fixes: 8d425b19b305 ("ethtool: set wake-on-lan settings with WOL_SET request")
 
> Reported-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 20:43 ` Jakub Kicinski
@ 2020-03-22 20:51   ` Michal Kubecek
  2020-03-22 21:06     ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Michal Kubecek @ 2020-03-22 20:51 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David S. Miller, netdev, Andrew Lunn, linux-kernel

On Sun, Mar 22, 2020 at 01:43:56PM -0700, Jakub Kicinski wrote:
> On Sun, 22 Mar 2020 21:15:51 +0100 (CET) Michal Kubecek wrote:
> > Andrew noticed that some handlers for *_SET commands leak a netdev
> > reference if required ethtool_ops callbacks do not exist. A simple
> > reproducer would be e.g.
> > 
> >   ip link add veth1 type veth peer name veth2
> >   ethtool -s veth1 wol g
> >   ip link del veth1
> > 
> > Make sure dev_put() is called when ethtool_ops check fails.
> 
> Fixes: e54d04e3afea ("ethtool: set message mask with DEBUG_SET request")
> Fixes: a53f3d41e4d3 ("ethtool: set link settings with LINKINFO_SET request")
> Fixes: bfbcfe2032e7 ("ethtool: set link modes related data with LINKMODES_SET request")
> Fixes: 8d425b19b305 ("ethtool: set wake-on-lan settings with WOL_SET request")

Yes, thank you, I forgot about Fixes tags.

Should I resubmit or will patchworks pick the tags from your reply?

Michal

> > Reported-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> 
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>

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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 20:51   ` Michal Kubecek
@ 2020-03-22 21:06     ` Jakub Kicinski
  2020-03-22 21:25       ` Michal Kubecek
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-03-22 21:06 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: David S. Miller, netdev, Andrew Lunn, linux-kernel

On Sun, 22 Mar 2020 21:51:09 +0100 Michal Kubecek wrote:
> On Sun, Mar 22, 2020 at 01:43:56PM -0700, Jakub Kicinski wrote:
> > On Sun, 22 Mar 2020 21:15:51 +0100 (CET) Michal Kubecek wrote:  
> > > Andrew noticed that some handlers for *_SET commands leak a netdev
> > > reference if required ethtool_ops callbacks do not exist. A simple
> > > reproducer would be e.g.
> > > 
> > >   ip link add veth1 type veth peer name veth2
> > >   ethtool -s veth1 wol g
> > >   ip link del veth1
> > > 
> > > Make sure dev_put() is called when ethtool_ops check fails.  
> > 
> > Fixes: e54d04e3afea ("ethtool: set message mask with DEBUG_SET request")
> > Fixes: a53f3d41e4d3 ("ethtool: set link settings with LINKINFO_SET request")
> > Fixes: bfbcfe2032e7 ("ethtool: set link modes related data with LINKMODES_SET request")
> > Fixes: 8d425b19b305 ("ethtool: set wake-on-lan settings with WOL_SET request")  
> 
> Yes, thank you, I forgot about Fixes tags.
> 
> Should I resubmit or will patchworks pick the tags from your reply?

Patchwork sees them, I think, but I don't think it adds them to the
patch as downloaded by git-pw. Probably easiest to repost.

> > > Reported-by: Andrew Lunn <andrew@lunn.ch>
> > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>  
> > 
> > Reviewed-by: Jakub Kicinski <kuba@kernel.org>  


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

* Re: [PATCH net] ethtool: fix reference leak in some *_SET handlers
  2020-03-22 21:06     ` Jakub Kicinski
@ 2020-03-22 21:25       ` Michal Kubecek
  0 siblings, 0 replies; 7+ messages in thread
From: Michal Kubecek @ 2020-03-22 21:25 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: David S. Miller, netdev, Andrew Lunn, linux-kernel

On Sun, Mar 22, 2020 at 02:06:23PM -0700, Jakub Kicinski wrote:
> On Sun, 22 Mar 2020 21:51:09 +0100 Michal Kubecek wrote:
> > On Sun, Mar 22, 2020 at 01:43:56PM -0700, Jakub Kicinski wrote:
> > > On Sun, 22 Mar 2020 21:15:51 +0100 (CET) Michal Kubecek wrote:  
> > > > Andrew noticed that some handlers for *_SET commands leak a netdev
> > > > reference if required ethtool_ops callbacks do not exist. A simple
> > > > reproducer would be e.g.
> > > > 
> > > >   ip link add veth1 type veth peer name veth2
> > > >   ethtool -s veth1 wol g
> > > >   ip link del veth1
> > > > 
> > > > Make sure dev_put() is called when ethtool_ops check fails.  
> > > 
> > > Fixes: e54d04e3afea ("ethtool: set message mask with DEBUG_SET request")
> > > Fixes: a53f3d41e4d3 ("ethtool: set link settings with LINKINFO_SET request")
> > > Fixes: bfbcfe2032e7 ("ethtool: set link modes related data with LINKMODES_SET request")
> > > Fixes: 8d425b19b305 ("ethtool: set wake-on-lan settings with WOL_SET request")  
> > 
> > Yes, thank you, I forgot about Fixes tags.
> > 
> > Should I resubmit or will patchworks pick the tags from your reply?
> 
> Patchwork sees them, I think, but I don't think it adds them to the
> patch as downloaded by git-pw. Probably easiest to repost.

Sent v2 with Fixes and Reviewed-by tags.

Michal

> > > > Reported-by: Andrew Lunn <andrew@lunn.ch>
> > > > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>  
> > > 
> > > Reviewed-by: Jakub Kicinski <kuba@kernel.org>  
> 

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

end of thread, other threads:[~2020-03-22 21:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-22 20:15 [PATCH net] ethtool: fix reference leak in some *_SET handlers Michal Kubecek
2020-03-22 20:23 ` Andrew Lunn
2020-03-22 20:42 ` Florian Fainelli
2020-03-22 20:43 ` Jakub Kicinski
2020-03-22 20:51   ` Michal Kubecek
2020-03-22 21:06     ` Jakub Kicinski
2020-03-22 21:25       ` Michal Kubecek

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).