Willem de Bruijn wrote:
> Paulos Yibelo wrote:
> > __virtio_net_hdr_to_skb() checks a minimum network-header length for
> > CHECKSUM_PARTIAL packets. Its checksum start is relative to skb->data,
> > but some callers have not established skb->network_header when they
> > convert the virtio header.
> >
> > Pass the data-relative L3 origin explicitly. Ethernet receive paths
> > parse the frame and nested VLAN headers without changing skb state.
> > AF_PACKET uses the frame's actual L3 origin even when the socket
> > protocol is ETH_P_IP and the raw frame carries VLAN tags. Non-Ethernet
> > AF_PACKET devices retain their established skb network offset.
> >
> > Also pass the actual L3 protocol so IPv6 packets use the 40-byte base
> > header minimum even without TCPv6 GSO. IFF_TUN obtains that protocol
> > from the packet before skb->protocol is set. Name the Ethernet parser
> > accordingly, use the same origin for tunnel validation, and propagate
> > conversion failures in UML.
> >
> > The bound remains a minimum; fragmentation paths separately validate
> > the parsed IPv4 or IPv6 header length before completing a checksum.
> >
> > Fixes: 49d14b54a527 ("net: test for not too small csum_start in
> > virtio_net_hdr_to_skb()")
> > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO
> > tunneling.")
> > Reported-by: Paulos Yibelo <[email protected]>
> > Link:
> > https://lore.kernel.org/netdev/[email protected]/
> > Cc: [email protected]
> > Assisted-by: LLM
> > Signed-off-by: Paulos Yibelo <[email protected]>
> > ---
> > arch/um/drivers/vector_transports.c | 13 ++++-
> > drivers/net/tun_vnet.h | 52 ++++++++++++++++-
> > drivers/net/virtio_net.c | 10 +++-
> > include/linux/virtio_net.h | 87 ++++++++++++++++++++++++-----
> > net/packet/af_packet.c | 24 +++++++-
> > 5 files changed, 163 insertions(+), 23 deletions(-)
>
> The fix may still miss the case IPv4 packets have options.
>
> This version is a very large patch.
>
> Untested shorter first suggestion by bot, which looks plausible as a
> starting point for discussion.
Cleaned up some more:
diff --git a/drivers/net/tun_vnet.h b/drivers/net/tun_vnet.h
index f4c652b1fa44..c24607af2aad 100644
--- a/drivers/net/tun_vnet.h
+++ b/drivers/net/tun_vnet.h
@@ -180,6 +180,9 @@ static inline int tun_vnet_hdr_put(int sz, struct
iov_iter *iter,
static inline int tun_vnet_hdr_to_skb(unsigned int flags, struct sk_buff
*skb,
const struct virtio_net_hdr *hdr)
{
+ if ((flags & TUN_TYPE_MASK) == IFF_TUN)
+ skb_reset_network_header(skb);
+
return virtio_net_hdr_to_skb(skb, hdr,
tun_vnet_is_little_endian(flags));
}
@@ -199,6 +202,9 @@ tun_vnet_hdr_tnl_to_skb(unsigned int flags,
netdev_features_t features,
struct sk_buff *skb,
const struct virtio_net_hdr_v1_hash_tunnel *hdr)
{
+ if ((flags & TUN_TYPE_MASK) == IFF_TUN)
+ skb_reset_network_header(skb);
+
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index c381b916c1b5..02c448de0802 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -48,6 +48,42 @@ static inline int virtio_net_hdr_set_proto(struct
sk_buff *skb,
return 0;
}
+static inline int virtio_net_hdr_nh_min_len(const struct sk_buff *skb,
+ unsigned int nh_min_len)
+{
+ int thoff = skb_transport_offset(skb);
+ __be16 proto;
+ int nhoff;
+
+ if (skb_network_header_was_set(skb)) {
+ nhoff = skb_network_offset(skb);
+ proto = skb->protocol;
+ } else {
+ if (unlikely(thoff < ETH_HLEN))
+ return -EINVAL;
+ nhoff = ETH_HLEN;
+ proto = eth_hdr(skb)->h_proto;
+ }
+
+ if (eth_type_vlan(proto)) {
+ proto = __vlan_get_protocol(skb, proto, &nhoff);
+ if (!proto)
+ return -EINVAL;
+ }
+
+ if (proto == htons(ETH_P_IP)) {
+ const struct iphdr *iph = (void *)(skb->data + nhoff);
+
+ if (unlikely(thoff < nhoff + sizeof(*iph)))
+ return -EINVAL;
+ nh_min_len = max_t(u32, iph->ihl * 4, sizeof(*iph));
+ } else if (proto == htons(ETH_P_IPV6)) {
+ nh_min_len = sizeof(struct ipv6hdr);
+ }
+
+ return nhoff + nh_min_len;
+}
+
static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
const struct virtio_net_hdr *hdr,
bool little_endian, u8 hdr_gso_type)
@@ -98,13 +134,15 @@ static inline int __virtio_net_hdr_to_skb(struct
sk_buff *skb,
u32 start = __virtio16_to_cpu(little_endian, hdr->csum_start);
u32 off = __virtio16_to_cpu(little_endian, hdr->csum_offset);
u32 needed = start + max_t(u32, thlen, off + sizeof(__sum16));
+ int min_thoff;
if (!pskb_may_pull(skb, needed))
return -EINVAL;
if (!skb_partial_csum_set(skb, start, off))
return -EINVAL;
- if (skb_transport_offset(skb) < nh_min_len)
+ min_thoff = virtio_net_hdr_nh_min_len(skb, nh_min_len);
+ if (min_thoff < 0 || skb_transport_offset(skb) < min_thoff)
return -EINVAL;
