netdev - re: drivers: net: xgene: fix statistics counters race condition

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 17 May 2016 10:33:44 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	isubramanian@....com
Cc:	netdev@...r.kernel.org
Subject: re: drivers: net: xgene: fix statistics counters race condition

Hello Iyappan Subramanian,

The patch 3bb502f83080: "drivers: net: xgene: fix statistics counters
race condition" from May 13, 2016, leads to the following static
checker warning:

	drivers/net/ethernet/apm/xgene/xgene_enet_main.c:487 xgene_enet_rx_frame()
	warn: should this be a bitwise op?

drivers/net/ethernet/apm/xgene/xgene_enet_main.c
   472          u8 status;
   473          int ret = 0;
   474  
   475          ndev = rx_ring->ndev;
   476          pdata = netdev_priv(ndev);
   477          dev = ndev_to_dev(rx_ring->ndev);
   478          buf_pool = rx_ring->buf_pool;
   479  
   480          dma_unmap_single(dev, GET_VAL(DATAADDR, le64_to_cpu(raw_desc->m1)),
   481                           XGENE_ENET_MAX_MTU, DMA_FROM_DEVICE);
   482          skb_index = GET_VAL(USERINFO, le64_to_cpu(raw_desc->m0));
   483          skb = buf_pool->rx_skb[skb_index];
   484  
   485          /* checking for error */
   486          status = (GET_VAL(ELERR, le64_to_cpu(raw_desc->m0)) << LERR_LEN) ||
   487                    GET_VAL(LERR, le64_to_cpu(raw_desc->m0));
   488          if (unlikely(status > 2)) {

This code doesn't make sense and I don't know what is intended.
LERR_LEN is 3, but why are we shifting when we just care about
true/false?  In the original code status could be > 2 but now it's
either 0 or 1.

   489                  dev_kfree_skb_any(skb);
   490                  xgene_enet_parse_error(rx_ring, netdev_priv(rx_ring->ndev),
   491                                         status);
   492                  ret = -EIO;
   493                  goto out;
   494          }

regards,
dan carpenter