| Message ID | 20170517202413.18321-1-bpoirier@suse.com |
|---|---|
| State | Accepted |
| Delegated to: | Jeff Kirsher |
| Headers | show |
From: Benjamin Poirier <bpoirier@suse.com> Date: Wed, 17 May 2017 16:24:13 -0400 > Some statistics passed to ethtool are garbage because e1000e_get_stats64() > doesn't write them, for example: tx_heartbeat_errors. This leaks kernel > memory to userspace and confuses users. > > Do like ixgbe and use dev_get_stats() which first zeroes out > rtnl_link_stats64. > > Fixes: 5944701df90d ("net: remove useless memset's in drivers get_stats64") > Reported-by: Stefan Priebe <s.priebe@profihost.ag> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com> Jeff, please be sure to pick this up, thanks.
On Thu, 2017-05-18 at 10:46 -0400, David Miller wrote: > From: Benjamin Poirier <bpoirier@suse.com> > Date: Wed, 17 May 2017 16:24:13 -0400 > > > Some statistics passed to ethtool are garbage because > > e1000e_get_stats64() > > doesn't write them, for example: tx_heartbeat_errors. This leaks kernel > > memory to userspace and confuses users. > > > > Do like ixgbe and use dev_get_stats() which first zeroes out > > rtnl_link_stats64. > > > > Fixes: 5944701df90d ("net: remove useless memset's in drivers > > get_stats64") > > Reported-by: Stefan Priebe <s.priebe@profihost.ag> > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com> > > Jeff, please be sure to pick this up, thanks. Yep, I have it in my tree, thanks.
> From: Kirsher, Jeffrey T > Sent: Friday, May 19, 2017 1:17 AM > To: David Miller <davem@davemloft.net>; bpoirier@suse.com > Cc: s.priebe@profihost.ag; intel-wired-lan@lists.osuosl.org; > netdev@vger.kernel.org; pmenzel@molgen.mpg.de; Neftin, Sasha > <sasha.neftin@intel.com>; Brown, Aaron F <aaron.f.brown@intel.com>; > stephen@networkplumber.org > Subject: Re: [PATCH v2] e1000e: Don't return uninitialized stats > > On Thu, 2017-05-18 at 10:46 -0400, David Miller wrote: > > From: Benjamin Poirier <bpoirier@suse.com> > > Date: Wed, 17 May 2017 16:24:13 -0400 > > > > > Some statistics passed to ethtool are garbage because > > > e1000e_get_stats64() > > > doesn't write them, for example: tx_heartbeat_errors. This leaks kernel > > > memory to userspace and confuses users. > > > > > > Do like ixgbe and use dev_get_stats() which first zeroes out > > > rtnl_link_stats64. > > > > > > Fixes: 5944701df90d ("net: remove useless memset's in drivers > > > get_stats64") > > > Reported-by: Stefan Priebe <s.priebe@profihost.ag> > > > Signed-off-by: Benjamin Poirier <bpoirier@suse.com> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c index e23dbd9190d6..5b4d97570896 100644 --- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -2072,7 +2072,7 @@ static void e1000_get_ethtool_stats(struct net_device *netdev, pm_runtime_get_sync(netdev->dev.parent); - e1000e_get_stats64(netdev, &net_stats); + dev_get_stats(netdev, &net_stats); pm_runtime_put_sync(netdev->dev.parent);
Some statistics passed to ethtool are garbage because e1000e_get_stats64() doesn't write them, for example: tx_heartbeat_errors. This leaks kernel memory to userspace and confuses users. Do like ixgbe and use dev_get_stats() which first zeroes out rtnl_link_stats64. Fixes: 5944701df90d ("net: remove useless memset's in drivers get_stats64") Reported-by: Stefan Priebe <s.priebe@profihost.ag> Signed-off-by: Benjamin Poirier <bpoirier@suse.com> --- Notes: Changes v1->v2: * add the Fixes tag drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)