Message ID | 20110317163720.GA26284@core.hellgate.ch |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2011-03-17 at 17:37 +0100, Roger Luethi wrote: > __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 <rl@hellgate.ch> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> Michał, was this just an oversight or is there some reason why we shouldn't check set_sg immediately? Ben. > --- > > Bug verified. Patch only compile-tested. > > net/core/ethtool.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index c1a71bb..a1086fb 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -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; >
2011/3/17 Roger Luethi <rl@hellgate.ch>: > __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 <rl@hellgate.ch> > --- > > Bug verified. Patch only compile-tested. > > net/core/ethtool.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c > index c1a71bb..a1086fb 100644 > --- a/net/core/ethtool.c > +++ b/net/core/ethtool.c > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Ben Hutchings <bhutchings@solarflare.com> Date: Thu, 17 Mar 2011 16:52:13 +0000 > On Thu, 2011-03-17 at 17:37 +0100, Roger Luethi wrote: >> __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 <rl@hellgate.ch> > Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/ethtool.c b/net/core/ethtool.c index c1a71bb..a1086fb 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -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;
__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 <rl@hellgate.ch> --- Bug verified. Patch only compile-tested. net/core/ethtool.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)