1. Patchew
  2. linux
  3. [PATCH net 0/4] virtio_net: Make RSS interact properly with queue number
  4. View patch

[PATCH net 1/4] virtio_net: Support dynamic rss indirection table size

Philo Lu posted 4 patches 2 weeks, 6 days ago
[PATCH net 1/4] virtio_net: Support dynamic rss indirection table size
Posted by Philo Lu 2 weeks, 6 days ago 
When reading/writing virtio_net_ctrl_rss, we get the indirection table
size from vi->rss_indir_table_size, which is initialized in
virtnet_probe(). However, the actual size of indirection_table was set
as VIRTIO_NET_RSS_MAX_TABLE_LEN=128. This collision may cause issues if
the vi->rss_indir_table_size exceeds 128.

This patch instead uses dynamic indirection table, allocated with
vi->rss after vi->rss_indir_table_size initialized. And free it in
virtnet_remove().

In virtnet_commit_rss_command(), sgs for rss is initialized differently
with hash_report. So indirection_table is not used if !vi->has_rss, and
then we don't need to alloc indirection_table for hash_report only uses.

Fixes: c7114b1249fa ("drivers/net/virtio_net: Added basic RSS support.")
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 869586c17ffd..75c1ff4efd13 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -368,15 +368,16 @@ struct receive_queue {
  * because table sizes may be differ according to the device configuration.
  */
 #define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
-#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
 struct virtio_net_ctrl_rss {
 	u32 hash_types;
 	u16 indirection_table_mask;
 	u16 unclassified_queue;
-	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+	u16 hash_cfg_reserved; /* for HASH_CONFIG (see virtio_net_hash_config for details) */
 	u16 max_tx_vq;
 	u8 hash_key_length;
 	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+
+	u16 *indirection_table;
 };
 
 /* Control VQ buffers: protected by the rtnl lock */
@@ -512,6 +513,25 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
 					       struct page *page, void *buf,
 					       int len, int truesize);
 
+static int rss_indirection_table_alloc(struct virtio_net_ctrl_rss *rss, u16 indir_table_size)
+{
+	if (!indir_table_size) {
+		rss->indirection_table = NULL;
+		return 0;
+	}
+
+	rss->indirection_table = kmalloc_array(indir_table_size, sizeof(u16), GFP_KERNEL);
+	if (!rss->indirection_table)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void rss_indirection_table_free(struct virtio_net_ctrl_rss *rss)
+{
+	kfree(rss->indirection_table);
+}
+
 static bool is_xdp_frame(void *ptr)
 {
 	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
@@ -3828,11 +3848,15 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
 	/* prepare sgs */
 	sg_init_table(sgs, 4);
 
-	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, indirection_table);
+	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, hash_cfg_reserved);
 	sg_set_buf(&sgs[0], &vi->rss, sg_buf_size);
 
-	sg_buf_size = sizeof(uint16_t) * (vi->rss.indirection_table_mask + 1);
-	sg_set_buf(&sgs[1], vi->rss.indirection_table, sg_buf_size);
+	if (vi->has_rss) {
+		sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+		sg_set_buf(&sgs[1], vi->rss.indirection_table, sg_buf_size);
+	} else {
+		sg_set_buf(&sgs[1], &vi->rss.hash_cfg_reserved, sizeof(uint16_t));
+	}
 
 	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, key)
 			- offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
@@ -6420,6 +6444,9 @@ static int virtnet_probe(struct virtio_device *vdev)
 			virtio_cread16(vdev, offsetof(struct virtio_net_config,
 				rss_max_indirection_table_length));
 	}
+	err = rss_indirection_table_alloc(&vi->rss, vi->rss_indir_table_size);
+	if (err)
+		goto free;
 
 	if (vi->has_rss || vi->has_rss_hash_report) {
 		vi->rss_key_size =
@@ -6674,6 +6701,8 @@ static void virtnet_remove(struct virtio_device *vdev)
 
 	remove_vq_common(vi);
 
+	rss_indirection_table_free(&vi->rss);
+
 	free_netdev(vi->dev);
 }
 
-- 
2.32.0.3.g01195cf9f
Re: [PATCH net 1/4] virtio_net: Support dynamic rss indirection table size
Posted by Joe Damato 2 weeks, 4 days ago 
On Mon, Nov 04, 2024 at 04:57:03PM +0800, Philo Lu wrote:
> When reading/writing virtio_net_ctrl_rss, we get the indirection table
> size from vi->rss_indir_table_size, which is initialized in
> virtnet_probe(). However, the actual size of indirection_table was set
> as VIRTIO_NET_RSS_MAX_TABLE_LEN=128. This collision may cause issues if
> the vi->rss_indir_table_size exceeds 128.
> 
> This patch instead uses dynamic indirection table, allocated with
> vi->rss after vi->rss_indir_table_size initialized. And free it in
> virtnet_remove().
> 
> In virtnet_commit_rss_command(), sgs for rss is initialized differently
> with hash_report. So indirection_table is not used if !vi->has_rss, and
> then we don't need to alloc indirection_table for hash_report only uses.
> 
> Fixes: c7114b1249fa ("drivers/net/virtio_net: Added basic RSS support.")
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>  drivers/net/virtio_net.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 869586c17ffd..75c1ff4efd13 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -368,15 +368,16 @@ struct receive_queue {
>   * because table sizes may be differ according to the device configuration.
>   */
>  #define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> -#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
>  struct virtio_net_ctrl_rss {
>  	u32 hash_types;
>  	u16 indirection_table_mask;
>  	u16 unclassified_queue;
> -	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +	u16 hash_cfg_reserved; /* for HASH_CONFIG (see virtio_net_hash_config for details) */
>  	u16 max_tx_vq;
>  	u8 hash_key_length;
>  	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +
> +	u16 *indirection_table;
>  };
>  
>  /* Control VQ buffers: protected by the rtnl lock */
> @@ -512,6 +513,25 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
>  					       struct page *page, void *buf,
>  					       int len, int truesize);
>  
> +static int rss_indirection_table_alloc(struct virtio_net_ctrl_rss *rss, u16 indir_table_size)
> +{
> +	if (!indir_table_size) {
> +		rss->indirection_table = NULL;
> +		return 0;
> +	}
> +
> +	rss->indirection_table = kmalloc_array(indir_table_size, sizeof(u16), GFP_KERNEL);
> +	if (!rss->indirection_table)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void rss_indirection_table_free(struct virtio_net_ctrl_rss *rss)
> +{
> +	kfree(rss->indirection_table);
> +}
> +
>  static bool is_xdp_frame(void *ptr)
>  {
>  	return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> @@ -3828,11 +3848,15 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
>  	/* prepare sgs */
>  	sg_init_table(sgs, 4);
>  
> -	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, indirection_table);
> +	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, hash_cfg_reserved);
>  	sg_set_buf(&sgs[0], &vi->rss, sg_buf_size);
>  
> -	sg_buf_size = sizeof(uint16_t) * (vi->rss.indirection_table_mask + 1);
> -	sg_set_buf(&sgs[1], vi->rss.indirection_table, sg_buf_size);
> +	if (vi->has_rss) {
> +		sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> +		sg_set_buf(&sgs[1], vi->rss.indirection_table, sg_buf_size);
> +	} else {
> +		sg_set_buf(&sgs[1], &vi->rss.hash_cfg_reserved, sizeof(uint16_t));
> +	}
>  
>  	sg_buf_size = offsetof(struct virtio_net_ctrl_rss, key)
>  			- offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> @@ -6420,6 +6444,9 @@ static int virtnet_probe(struct virtio_device *vdev)
>  			virtio_cread16(vdev, offsetof(struct virtio_net_config,
>  				rss_max_indirection_table_length));
>  	}
> +	err = rss_indirection_table_alloc(&vi->rss, vi->rss_indir_table_size);
> +	if (err)
> +		goto free;
>  
>  	if (vi->has_rss || vi->has_rss_hash_report) {
>  		vi->rss_key_size =
> @@ -6674,6 +6701,8 @@ static void virtnet_remove(struct virtio_device *vdev)
>  
>  	remove_vq_common(vi);
>  
> +	rss_indirection_table_free(&vi->rss);
> +
>  	free_netdev(vi->dev);
>  }
>

I'm not an expert on virtio, so I don't feel comfortable giving a
Reviewed-by, but this does seem to fix a potential out of bounds
access in virtnet_init_default_rss if rss_indir_table_size were
larger than VIRTIO_NET_RSS_MAX_TABLE_LEN (128).

Acked-by: Joe Damato <jdamato@fastly.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.