lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 13 May 2016 18:41:44 -0700 From: Eric Dumazet <eric.dumazet@...il.com> To: Iyappan Subramanian <isubramanian@....com> Cc: davem@...emloft.net, netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, patches@....com, toanle@....com Subject: Re: [PATCH v2 4/5] drivers: net: xgene: fix statistics counters race condition On Fri, 2016-05-13 at 16:53 -0700, Iyappan Subramanian wrote: > This patch fixes the race condition on updating the statistics > counters by moving the counters to the ring structure. > ... > @@ -1127,12 +1127,31 @@ static struct rtnl_link_stats64 *xgene_enet_get_stats64( > { > struct xgene_enet_pdata *pdata = netdev_priv(ndev); > struct rtnl_link_stats64 *stats = &pdata->stats; > + struct xgene_enet_desc_ring *ring; > + int i; > + > + memset(stats, 0, sizeof(struct rtnl_link_stats64)); The memset() should already be done in the caller. > + for (i = 0; i < pdata->txq_cnt; i++) { > + ring = pdata->tx_ring[i]; > + if (ring) { > + stats->tx_packets += ring->tx_packets; > + stats->tx_bytes += ring->tx_bytes; > + } > + } > > - stats->rx_errors += stats->rx_length_errors + > - stats->rx_crc_errors + > - stats->rx_frame_errors + > - stats->rx_fifo_errors; Right, this piece was completely buggy. Not a race, but rx_errors was growing every time stats were fetched on this device. stats->rx_errors = sum(...) was probably the intent. > - memcpy(storage, &pdata->stats, sizeof(struct rtnl_link_stats64)); > + for (i = 0; i < pdata->rxq_cnt; i++) { > + ring = pdata->rx_ring[i]; > + if (ring) { > + stats->rx_packets += ring->rx_packets; > + stats->rx_bytes += ring->rx_bytes; > + stats->rx_errors += ring->rx_length_errors + > + ring->rx_crc_errors + > + ring->rx_frame_errors + > + ring->rx_fifo_errors; > + stats->rx_dropped += ring->rx_dropped; > + } > + } > + memcpy(storage, stats, sizeof(struct rtnl_link_stats64)); > > return storage; > } > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > index cc40c30..092fbec 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.h > @@ -121,6 +121,16 @@ struct xgene_enet_desc_ring { > struct xgene_enet_raw_desc16 *raw_desc16; > }; > __le64 *exp_bufs; > + u64 tx_packets; > + u64 tx_bytes; > + u64 rx_packets; > + u64 rx_bytes; > + u64 rx_dropped; > + u64 rx_errors; > + u64 rx_length_errors; > + u64 rx_crc_errors; > + u64 rx_frame_errors; > + u64 rx_fifo_errors; Note that there is still a race on 32bit arches. It seems this driver should really use "unsigned long" counters and provide standard ndo_get_stats()
Powered by blists - more mailing lists