Message ID | 20170424190151.cbmbbeyjspoolpho@f1.synalogic.ca |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
From: Benjamin Poirier <bpoirier@suse.com> Date: Mon, 24 Apr 2017 12:01:51 -0700 > On 2017/04/24 10:23, Paul Menzel wrote: >> Dear Benjamin, >> >> >> Thank you for your fix. >> >> On 04/21/17 23:20, Benjamin Poirier wrote: >> > 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. >> >> Could you please give specific examples to reproduce the issue? That way >> your fix can also be tested. >> > > Some fields in e1000_get_ethtool_stats()'s net_stats are not initialized > by e1000e_get_stats64(). The structure is allocated on the stack, > therefore, the value of those fields depends on previous stack content; > that in turns depends on kernel version, compiler and previous execution > path. I agree. I read over these code paths earlier today and came to the same exact conclusion.
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c @@ -2061,6 +2061,8 @@ static void e1000_get_ethtool_stats(struct net_device *netdev, int i; char *p = NULL; + memset(&net_stats, 0xff, sizeof(net_stats)); + pm_runtime_get_sync(netdev->dev.parent); e1000e_get_stats64(netdev, &net_stats);