ethtool: __ethtool_set_sg: check for function pointer before using it

PATCH Applied Roger Luethi 4 participants 4 messages Mar 17, 2011 – Mar 18, 2011

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Roger Luethi

author

2011-03-17 16:37 UTC #0 lore raw

PATCH 1/1 ethtool: __ethtool_set_sg: check for function pointer before using it

__ethtool_set_sg does not check if dev->ethtool_ops->set_sg is defined which can result in a NULL pointer dereference when ethtool is used to change SG settings for drivers without SG support.

Signed-off-by Roger Luethi <[email protected]>

---

Bug verified. Patch only compile-tested.

 net/core/ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

net/core/ethtool.c +3 -0

index c1a71bb..a1086fb 100644
@@ -1457,6 +1457,9 @@ static int __ethtool_set_sg(struct net_device *dev, u32 data)
 {
 	int err;
 
+	if (!dev->ethtool_ops->set_sg)
+		return -EOPNOTSUPP;
+
 	if (data && !(dev->features & NETIF_F_ALL_CSUM))
 		return -EINVAL;

Signature · 1 line

1.7.3.4

Ben Hutchings

Reviewed-by on v1 1/1

2011-03-17 16:52 UTC #1 lore raw

Quoted 5 lines · Roger Luethi

__ethtool_set_sg does not check if dev->ethtool_ops->set_sg is defined which can result in a NULL pointer dereference when ethtool is used to change SG settings for drivers without SG support.
Signed-off-by Roger Luethi <[email protected]>

Reviewed-by Ben Hutchings <[email protected]>

Michał, was this just an oversight or is there some reason why we shouldn't check set_sg immediately?

Ben.

Quoted 21 lines

---

Bug verified. Patch only compile-tested.

 net/core/ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

net/core/ethtool.c +3 -0

index c1a71bb..a1086fb 100644
@@ -1457,6 +1457,9 @@ static int __ethtool_set_sg(struct net_device *dev, u32 data)
 {
 	int err;
 
+	if (!dev->ethtool_ops->set_sg)
+		return -EOPNOTSUPP;
+
 	if (data && !(dev->features & NETIF_F_ALL_CSUM))
 		return -EINVAL;

Signature · 3 lines

Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

David Miller

maintainer Applied

2011-03-18 22:15 UTC #2 lore raw

From: Ben Hutchings <[email protected]>
Date: Thu, 17 Mar 2011 16:52:13 +0000

Quoted 7 lines · Roger Luethi

Quoted 5 lines
__ethtool_set_sg does not check if dev->ethtool_ops->set_sg is defined which can result in a NULL pointer dereference when ethtool is used to change SG settings for drivers without SG support.
Signed-off-by Roger Luethi <[email protected]>
Reviewed-by Ben Hutchings <[email protected]>

Applied.

Michał Mirosław

2011-03-17 17:01 UTC #3 lore raw

In reply to Roger Luethi

2011/3/17 Roger Luethi <[email protected]>:

Quoted 17 lines

__ethtool_set_sg does not check if dev->ethtool_ops->set_sg is defined which can result in a NULL pointer dereference when ethtool is used to change SG settings for drivers without SG support.
Signed-off-by Roger Luethi <[email protected]>
---
Bug verified. Patch only compile-tested.
net/core/ethtool.c | 3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
net/core/ethtool.c +0 -0
index c1a71bb..a1086fb 100644

net/core/ethtool.c static int __ethtool_set_sg(struct net_device *dev, u32 data) · line 1465

Quoted 9 lines

net/core/ethtool.c +3 -0

@@ -1457,6 +1457,9 @@ static int __ethtool_set_sg(struct net_device *dev, u32 data)
  {
        int err;
 
+       if (!dev->ethtool_ops->set_sg)
+               return -EOPNOTSUPP;
+
        if (data && !(dev->features & NETIF_F_ALL_CSUM))
                return -EINVAL;

Yes. __ethtool_set_sg() is the only function that was already there before my unification series and I did tests only on drivers which had set_sg() defined. :-/

This should go into 2.6.39 as a bugfix (adding Cc: DaveM).

Best Regards,
Michał Mirosław