mirror of
https://github.com/torvalds/linux.git
synced 2026-10-23 23:51:02 +02:00
net: always dissect GSO packets in __virtio_net_hdr_to_skb()
Commit9e8db59132("net: avoid false positives in untrusted gso validation") added a '&& skb->network_header' check before flow-dissecting GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM in __virtio_net_hdr_to_skb(), because some callers (such as tun_get_user(), tun_xdp_one(), virtnet_receive_done(), and raw_verify_header()) called virtio_net_hdr_*_to_skb() before initializing skb->network_header and skb->dev. Because __alloc_skb() and __build_skb_around() zero-initialize skb->network_header to 0 (unlike mac_header and transport_header which are initialized to ~0U), those four callers always had skb->network_header == 0 and bypassed flow dissection in __virtio_net_hdr_to_skb(). More generally, skb->network_header is an offset from skb->head (where 0 is also a valid offset whenever skb_headroom(skb) is 0), not a boolean flag. Whenever the 'if (gso_type && skb->network_header)' branch was skipped, the fallback 'else if (gso_type)' only pulled nh_min_len + thlen (40 bytes for TCPv4) without dissecting the packet, without validating ip_proto or n_proto, and without setting skb->transport_header. If the packet has a malformed network header, it is not rejected and a subsequent skb_probe_transport_header() also fails, leaving skb->transport_header at ~0U (0xffff). Similarly, if an IPv4 packet carries IP options (ihl > 5) or an IPv6 packet carries extension headers, pulling only nh_min_len + thlen can leave the TCP header outside skb->head. In both cases, tcp_hdrlen(skb) in skb_gso_transport_seglen() reads out-of-bounds: BUG: KASAN: slab-out-of-bounds in skb_gso_transport_seglen Read of size 2 by task poc/133 skb_gso_transport_seglen (net/core/gso.c:155) skb_gso_validate_mac_len (net/core/gso.c:270) tbf_enqueue (net/sched/sch_tbf.c:260) dev_qdisc_enqueue (net/core/dev.c:4227) __dev_queue_xmit (net/core/dev.c:4884) In addition, checking virtio_net_hdr_match_proto() only inside 'if (!skb->protocol)' before flow dissection both skipped validation when skb->protocol was pre-set by the caller and rejected VLAN-tagged frames whose outer L2 protocol is ETH_P_8021Q or ETH_P_8021AD. Fix this by: 1. Initializing skb->dev and skb->network_header (plus skb->protocol for IFF_TUN) before virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(), virtnet_receive_done(), and raw_verify_header(). In tun_get_user(), drop the redundant skb_reset_mac_header(skb) in the IFF_TUN case since __virtio_net_hdr_to_skb() unconditionally resets mac_header. 2. Removing '&& skb->network_header' and the unvalidated 'else if (gso_type)' fallback in __virtio_net_hdr_to_skb() so all GSO packets without VIRTIO_NET_HDR_F_NEEDS_CSUM are flow-dissected, have their transport header pulled into linear data, and have skb->transport_header set. 3. Moving the virtio_net_hdr_match_proto() check to after skb_flow_dissect_flow_keys_basic(), validating keys.basic.n_proto against hdr_gso_type. Fixes:9e8db59132("net: avoid false positives in untrusted gso validation") Fixes:d5be7f632b("net: validate untrusted gso packets without csum offload") Fixes:924a9bc362("net: check if protocol extracted by virtio_net_hdr_set_proto is correct") Reported-by: Weiming Shi <bestswngs@gmail.com> Closes: https://lore.kernel.org/netdev/20260927163117.746432-2-bestswngs@gmail.com/ Signed-off-by: Eric Dumazet <edumazet@kernel.org> Reviewed-by: Willem de Bruijn <willemb@google.com> Cc: Michael S. Tsirkin <mst@redhat.com> Link: https://patch.msgid.link/20261001191140.2818991-3-edumazet@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
parent
99bda1ecbd
commit
44378d02c5
|
|
@ -209,6 +209,7 @@ static int raw_verify_header(
|
|||
if ((vheader->flags & VIRTIO_NET_HDR_F_DATA_VALID) > 0)
|
||||
return 1;
|
||||
|
||||
skb_set_network_header(skb, ETH_HLEN);
|
||||
virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian());
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1897,12 +1897,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
|
|||
}
|
||||
}
|
||||
|
||||
if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
|
||||
atomic_long_inc(&tun->rx_frame_errors);
|
||||
err = -EINVAL;
|
||||
goto free_skb;
|
||||
}
|
||||
|
||||
skb->dev = tun->dev;
|
||||
switch (tun->flags & TUN_TYPE_MASK) {
|
||||
case IFF_TUN:
|
||||
if (tun->flags & IFF_NO_PI) {
|
||||
|
|
@ -1927,9 +1922,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
|
|||
}
|
||||
}
|
||||
|
||||
skb_reset_mac_header(skb);
|
||||
skb_reset_network_header(skb);
|
||||
skb->protocol = pi.proto;
|
||||
skb->dev = tun->dev;
|
||||
break;
|
||||
case IFF_TAP:
|
||||
if (!pskb_may_pull(skb, ETH_HLEN)) {
|
||||
|
|
@ -1937,10 +1931,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
|
|||
drop_reason = SKB_DROP_REASON_HDR_TRUNC;
|
||||
goto drop;
|
||||
}
|
||||
skb->protocol = eth_type_trans(skb, tun->dev);
|
||||
skb_set_network_header(skb, ETH_HLEN);
|
||||
break;
|
||||
}
|
||||
|
||||
if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) {
|
||||
atomic_long_inc(&tun->rx_frame_errors);
|
||||
err = -EINVAL;
|
||||
goto free_skb;
|
||||
}
|
||||
|
||||
if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP)
|
||||
skb->protocol = eth_type_trans(skb, tun->dev);
|
||||
|
||||
/* copy skb_ubuf_info for callback when skb has no error */
|
||||
if (zerocopy) {
|
||||
skb_zcopy_init(skb, msg_control);
|
||||
|
|
@ -2600,6 +2603,8 @@ static int tun_xdp_one(struct tun_struct *tun,
|
|||
|
||||
features = tun_vnet_hdr_guest_features(READ_ONCE(tun->vnet_hdr_sz));
|
||||
tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso;
|
||||
skb->dev = tun->dev;
|
||||
skb_set_network_header(skb, ETH_HLEN);
|
||||
if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) {
|
||||
atomic_long_inc(&tun->rx_frame_errors);
|
||||
kfree_skb(skb);
|
||||
|
|
|
|||
|
|
@ -2515,6 +2515,8 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
|
|||
goto frame_err;
|
||||
}
|
||||
|
||||
skb->dev = dev;
|
||||
skb_set_network_header(skb, ETH_HLEN);
|
||||
if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl,
|
||||
vi->rx_tnl_csum,
|
||||
virtio_is_little_endian(vi->vdev))) {
|
||||
|
|
|
|||
|
|
@ -111,48 +111,38 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
|
|||
p_off = nh_min_len + thlen;
|
||||
if (!pskb_may_pull(skb, p_off))
|
||||
return -EINVAL;
|
||||
} else {
|
||||
} else if (gso_type) {
|
||||
/* gso packets without NEEDS_CSUM do not set transport_offset.
|
||||
* probe and drop if does not match one of the above types.
|
||||
*/
|
||||
if (gso_type && skb->network_header) {
|
||||
struct flow_keys_basic keys;
|
||||
struct flow_keys_basic keys;
|
||||
|
||||
if (!skb->protocol) {
|
||||
__be16 protocol = dev_parse_header_protocol(skb);
|
||||
|
||||
if (!protocol)
|
||||
virtio_net_hdr_set_proto(skb, hdr);
|
||||
else if (!virtio_net_hdr_match_proto(protocol,
|
||||
hdr_gso_type))
|
||||
return -EINVAL;
|
||||
else
|
||||
skb->protocol = protocol;
|
||||
}
|
||||
retry:
|
||||
if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
|
||||
NULL, 0, 0, 0,
|
||||
0)) {
|
||||
/* UFO does not specify ipv4 or 6: try both */
|
||||
if (gso_type & SKB_GSO_UDP &&
|
||||
skb->protocol == htons(ETH_P_IP)) {
|
||||
skb->protocol = htons(ETH_P_IPV6);
|
||||
goto retry;
|
||||
}
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
p_off = keys.control.thoff + thlen;
|
||||
if (!pskb_may_pull(skb, p_off) ||
|
||||
keys.basic.ip_proto != ip_proto)
|
||||
return -EINVAL;
|
||||
|
||||
skb_set_transport_header(skb, keys.control.thoff);
|
||||
} else if (gso_type) {
|
||||
p_off = nh_min_len + thlen;
|
||||
if (!pskb_may_pull(skb, p_off))
|
||||
return -EINVAL;
|
||||
if (!skb->protocol) {
|
||||
skb->protocol = dev_parse_header_protocol(skb);
|
||||
if (!skb->protocol)
|
||||
virtio_net_hdr_set_proto(skb, hdr);
|
||||
}
|
||||
retry:
|
||||
if (!skb_flow_dissect_flow_keys_basic(NULL, skb, &keys,
|
||||
NULL, 0, 0, 0,
|
||||
0)) {
|
||||
/* UFO does not specify ipv4 or 6: try both */
|
||||
if (gso_type & SKB_GSO_UDP &&
|
||||
skb->protocol == htons(ETH_P_IP)) {
|
||||
skb->protocol = htons(ETH_P_IPV6);
|
||||
goto retry;
|
||||
}
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
p_off = keys.control.thoff + thlen;
|
||||
if (!pskb_may_pull(skb, p_off) ||
|
||||
keys.basic.ip_proto != ip_proto ||
|
||||
!virtio_net_hdr_match_proto(keys.basic.n_proto,
|
||||
hdr_gso_type))
|
||||
return -EINVAL;
|
||||
|
||||
skb_set_transport_header(skb, keys.control.thoff);
|
||||
}
|
||||
|
||||
if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user