Skip to content

Commit dae6474

Browse files
Feng Liudavem330
authored andcommitted
virtio_net: Introduce skb_vnet_common_hdr to avoid typecasting
The virtio_net driver currently deals with different versions and types of virtio net headers, such as virtio_net_hdr_mrg_rxbuf, virtio_net_hdr_v1_hash, etc. Due to these variations, the code relies on multiple type casts to convert memory between different structures, potentially leading to bugs when there are changes in these structures. Introduces the "struct skb_vnet_common_hdr" as a unifying header structure using a union. With this approach, various virtio net header structures can be converted by accessing different members of this structure, thus eliminating the need for type casting and reducing the risk of potential bugs. For example following code: static struct sk_buff *page_to_skb(struct virtnet_info *vi, struct receive_queue *rq, struct page *page, unsigned int offset, unsigned int len, unsigned int truesize, unsigned int headroom) { [...] struct virtio_net_hdr_mrg_rxbuf *hdr; [...] hdr_len = vi->hdr_len; [...] ok: hdr = skb_vnet_hdr(skb); memcpy(hdr, hdr_p, hdr_len); [...] } When VIRTIO_NET_F_HASH_REPORT feature is enabled, hdr_len = 20 But the sizeof(*hdr) is 12, memcpy(hdr, hdr_p, hdr_len); will copy 20 bytes to the hdr, which make a potential risk of bug. And this risk can be avoided by introducing struct skb_vnet_common_hdr. Change log v1->v2 feedback from Willem de Bruijn <willemdebruijn.kernel@gmail.com> feedback from Simon Horman <horms@kernel.org> 1. change to use net-next tree. 2. move skb_vnet_common_hdr inside kernel file instead of the UAPI header. v2->v3 feedback from Willem de Bruijn <willemdebruijn.kernel@gmail.com> 1. fix typo in commit message. 2. add original struct virtio_net_hdr into union 3. remove virtio_net_hdr_mrg_rxbuf variable in receive_buf; Signed-off-by: Feng Liu <feliu@nvidia.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Acked-by: Jason Wang <jasowang@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 45f9cb6 commit dae6474

File tree

1 file changed

+18
-9
lines changed

1 file changed

+18
-9
lines changed

drivers/net/virtio_net.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,14 @@ struct padded_vnet_hdr {
312312
char padding[12];
313313
};
314314

315+
struct virtio_net_common_hdr {
316+
union {
317+
struct virtio_net_hdr hdr;
318+
struct virtio_net_hdr_mrg_rxbuf mrg_hdr;
319+
struct virtio_net_hdr_v1_hash hash_v1_hdr;
320+
};
321+
};
322+
315323
static void virtnet_rq_free_unused_buf(struct virtqueue *vq, void *buf);
316324
static void virtnet_sq_free_unused_buf(struct virtqueue *vq, void *buf);
317325

@@ -353,9 +361,10 @@ static int rxq2vq(int rxq)
353361
return rxq * 2;
354362
}
355363

356-
static inline struct virtio_net_hdr_mrg_rxbuf *skb_vnet_hdr(struct sk_buff *skb)
364+
static inline struct virtio_net_common_hdr *
365+
skb_vnet_common_hdr(struct sk_buff *skb)
357366
{
358-
return (struct virtio_net_hdr_mrg_rxbuf *)skb->cb;
367+
return (struct virtio_net_common_hdr *)skb->cb;
359368
}
360369

361370
/*
@@ -478,7 +487,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
478487
unsigned int headroom)
479488
{
480489
struct sk_buff *skb;
481-
struct virtio_net_hdr_mrg_rxbuf *hdr;
490+
struct virtio_net_common_hdr *hdr;
482491
unsigned int copy, hdr_len, hdr_padded_len;
483492
struct page *page_to_free = NULL;
484493
int tailroom, shinfo_size;
@@ -563,7 +572,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
563572
give_pages(rq, page);
564573

565574
ok:
566-
hdr = skb_vnet_hdr(skb);
575+
hdr = skb_vnet_common_hdr(skb);
567576
memcpy(hdr, hdr_p, hdr_len);
568577
if (page_to_free)
569578
put_page(page_to_free);
@@ -975,7 +984,7 @@ static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
975984
return NULL;
976985

977986
buf += header_offset;
978-
memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
987+
memcpy(skb_vnet_common_hdr(skb), buf, vi->hdr_len);
979988

980989
return skb;
981990
}
@@ -1586,7 +1595,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
15861595
{
15871596
struct net_device *dev = vi->dev;
15881597
struct sk_buff *skb;
1589-
struct virtio_net_hdr_mrg_rxbuf *hdr;
1598+
struct virtio_net_common_hdr *hdr;
15901599

15911600
if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
15921601
pr_debug("%s: short packet %i\n", dev->name, len);
@@ -1606,9 +1615,9 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
16061615
if (unlikely(!skb))
16071616
return;
16081617

1609-
hdr = skb_vnet_hdr(skb);
1618+
hdr = skb_vnet_common_hdr(skb);
16101619
if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
1611-
virtio_skb_set_hash((const struct virtio_net_hdr_v1_hash *)hdr, skb);
1620+
virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);
16121621

16131622
if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
16141623
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2114,7 +2123,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
21142123
if (can_push)
21152124
hdr = (struct virtio_net_hdr_mrg_rxbuf *)(skb->data - hdr_len);
21162125
else
2117-
hdr = skb_vnet_hdr(skb);
2126+
hdr = &skb_vnet_common_hdr(skb)->mrg_hdr;
21182127

21192128
if (virtio_net_hdr_from_skb(skb, &hdr->hdr,
21202129
virtio_is_little_endian(vi->vdev), false,

0 commit comments

Comments
 (0)