netdev - Re: [PATCH net-next 2/3] virtio_net: avoid data-races on dev->stats fields

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1692599505.5924318-1-xuanzhuo@linux.alibaba.com>
Date: Mon, 21 Aug 2023 14:31:45 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "Michael S . Tsirkin" <mst@...hat.com>,
 Jason Wang <jasowang@...hat.com>,
 netdev@...r.kernel.org,
 eric.dumazet@...il.com,
 Eric Dumazet <edumazet@...gle.com>,
 syzbot <syzkaller@...glegroups.com>,
 "David S . Miller" <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 2/3] virtio_net: avoid data-races on dev->stats fields

On Sat, 19 Aug 2023 04:40:58 +0000, Eric Dumazet <edumazet@...gle.com> wrote:
> Use DEV_STATS_INC() and DEV_STATS_READ() which provide
> atomicity on paths that can be used concurrently.
>
> Reported-by: syzbot <syzkaller@...glegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>

Reviewed-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>

> Cc: "Michael S. Tsirkin" <mst@...hat.com>
> Cc: Jason Wang <jasowang@...hat.com>
> Cc: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 494242bb9cf60cfd54381f12eef338711a558483..d0570de1b0b47ee585ff8945a438e7cd2e1a7de5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1085,7 +1085,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  	if (unlikely(len > GOOD_PACKET_LEN)) {
>  		pr_debug("%s: rx error: len %u exceeds max size %d\n",
>  			 dev->name, len, GOOD_PACKET_LEN);
> -		dev->stats.rx_length_errors++;
> +		DEV_STATS_INC(dev, rx_length_errors);
>  		goto err;
>  	}
>
> @@ -1150,7 +1150,7 @@ static void mergeable_buf_free(struct receive_queue *rq, int num_buf,
>  		if (unlikely(!buf)) {
>  			pr_debug("%s: rx error: %d buffers missing\n",
>  				 dev->name, num_buf);
> -			dev->stats.rx_length_errors++;
> +			DEV_STATS_INC(dev, rx_length_errors);
>  			break;
>  		}
>  		stats->bytes += len;
> @@ -1259,7 +1259,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
>  			pr_debug("%s: rx error: %d buffers out of %d missing\n",
>  				 dev->name, *num_buf,
>  				 virtio16_to_cpu(vi->vdev, hdr->num_buffers));
> -			dev->stats.rx_length_errors++;
> +			DEV_STATS_INC(dev, rx_length_errors);
>  			goto err;
>  		}
>
> @@ -1278,7 +1278,7 @@ static int virtnet_build_xdp_buff_mrg(struct net_device *dev,
>  			put_page(page);
>  			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
>  				 dev->name, len, (unsigned long)(truesize - room));
> -			dev->stats.rx_length_errors++;
> +			DEV_STATS_INC(dev, rx_length_errors);
>  			goto err;
>  		}
>
> @@ -1457,7 +1457,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  	if (unlikely(len > truesize - room)) {
>  		pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
>  			 dev->name, len, (unsigned long)(truesize - room));
> -		dev->stats.rx_length_errors++;
> +		DEV_STATS_INC(dev, rx_length_errors);
>  		goto err_skb;
>  	}
>
> @@ -1489,7 +1489,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  				 dev->name, num_buf,
>  				 virtio16_to_cpu(vi->vdev,
>  						 hdr->num_buffers));
> -			dev->stats.rx_length_errors++;
> +			DEV_STATS_INC(dev, rx_length_errors);
>  			goto err_buf;
>  		}
>
> @@ -1503,7 +1503,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  		if (unlikely(len > truesize - room)) {
>  			pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
>  				 dev->name, len, (unsigned long)(truesize - room));
> -			dev->stats.rx_length_errors++;
> +			DEV_STATS_INC(dev, rx_length_errors);
>  			goto err_skb;
>  		}
>
> @@ -1590,7 +1590,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>
>  	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>  		pr_debug("%s: short packet %i\n", dev->name, len);
> -		dev->stats.rx_length_errors++;
> +		DEV_STATS_INC(dev, rx_length_errors);
>  		virtnet_rq_free_unused_buf(rq->vq, buf);
>  		return;
>  	}
> @@ -1630,7 +1630,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>  	return;
>
>  frame_err:
> -	dev->stats.rx_frame_errors++;
> +	DEV_STATS_INC(dev, rx_frame_errors);
>  	dev_kfree_skb(skb);
>  }
>
> @@ -2170,12 +2170,12 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>
>  	/* This should not happen! */
>  	if (unlikely(err)) {
> -		dev->stats.tx_fifo_errors++;
> +		DEV_STATS_INC(dev, tx_fifo_errors);
>  		if (net_ratelimit())
>  			dev_warn(&dev->dev,
>  				 "Unexpected TXQ (%d) queue failure: %d\n",
>  				 qnum, err);
> -		dev->stats.tx_dropped++;
> +		DEV_STATS_INC(dev, tx_dropped);
>  		dev_kfree_skb_any(skb);
>  		return NETDEV_TX_OK;
>  	}
> @@ -2394,10 +2394,10 @@ static void virtnet_stats(struct net_device *dev,
>  		tot->tx_errors  += terrors;
>  	}
>
> -	tot->tx_dropped = dev->stats.tx_dropped;
> -	tot->tx_fifo_errors = dev->stats.tx_fifo_errors;
> -	tot->rx_length_errors = dev->stats.rx_length_errors;
> -	tot->rx_frame_errors = dev->stats.rx_frame_errors;
> +	tot->tx_dropped = DEV_STATS_READ(dev, tx_dropped);
> +	tot->tx_fifo_errors = DEV_STATS_READ(dev, tx_fifo_errors);
> +	tot->rx_length_errors = DEV_STATS_READ(dev, rx_length_errors);
> +	tot->rx_frame_errors = DEV_STATS_READ(dev, rx_frame_errors);
>  }
>
>  static void virtnet_ack_link_announce(struct virtnet_info *vi)
> --
> 2.42.0.rc1.204.g551eb34607-goog
>