linux-kernel - [PATCH] net: tap: validate metadata and length for XDP buff before building up skb

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-next>] [day] [month] [year] [list]

Message-Id: <1717026141-25716-1-git-send-email-si-wei.liu@oracle.com>
Date: Wed, 29 May 2024 16:42:21 -0700
From: Si-Wei Liu <si-wei.liu@...cle.com>
To: willemdebruijn.kernel@...il.com, jasowang@...hat.com, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
        mst@...hat.com, boris.ostrovsky@...cle.com
Subject: [PATCH] net: tap: validate metadata and length for XDP buff before building up skb

The cited commit missed to check against the validity of the length
and various pointers on the XDP buff metadata in the tap_get_user_xdp()
path, which could cause a corrupted skb to be sent downstack. For
instance, tap_get_user() prohibits short frame which has the length
less than Ethernet header size from being transmitted, while the
skb_set_network_header() in tap_get_user_xdp() would set skb's
network_header regardless of the actual XDP buff data size. This
could either cause out-of-bound access beyond the actual length, or
confuse the underlayer with incorrect or inconsistent header length
in the skb metadata.

Propose to drop any frame shorter than the Ethernet header size just
like how tap_get_user() does. While at it, validate the pointers in
XDP buff to avoid potential size overrun.

Fixes: 0efac27791ee ("tap: accept an array of XDP buffs through sendmsg()")
Cc: jasowang@...hat.com
Signed-off-by: Si-Wei Liu <si-wei.liu@...cle.com>
---
 drivers/net/tap.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index bfdd3875fe86..69596479536f 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1177,6 +1177,13 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
 	struct sk_buff *skb;
 	int err, depth;

+	if (unlikely(xdp->data < xdp->data_hard_start ||
+		     xdp->data_end < xdp->data ||
+		     xdp->data_end - xdp->data < ETH_HLEN)) {
+		err = -EINVAL;
+		goto err;
+	}
+
 	if (q->flags & IFF_VNET_HDR)
 		vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);

-- 
2.39.3