net: psp: add more validation

Code checkers / AI scans will complain that we have already modified the packet by the time we realize that protocol is not IP.

Move the skb->protocol check to before skb_push()/memmove() so that the skb is not left in a corrupted state when the function returns false for an unsupported protocol. psp_dev_rcv() follows similar pattern.

Today this path is unreachable because both in-tree callers (mlx5 and netdevsim) only reach psp_dev_encapsulate() from TCP socket TX paths where skb->protocol is always ETH_P_IP or ETH_P_IPV6, and both drop the skb on a false return, anyway.

Signed-off-by Jakub Kicinski <[email protected]>

---

Code checkers / AI scans will complain that we have already modified the packet by the time we realize that protocol is not IP.

Move the skb->protocol check to before skb_push()/memmove() so that the skb is not left in a corrupted state when the function returns false for an unsupported protocol. psp_dev_rcv() follows similar pattern.

Today this path is unreachable because both in-tree callers (mlx5 and netdevsim) only reach psp_dev_encapsulate() from TCP socket TX paths where skb->protocol is always ETH_P_IP or ETH_P_IPV6, and both drop the skb on a false return, anyway.

Signed-off-by Jakub Kicinski <[email protected]>

---
 net/psp/psp_main.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

net/psp/psp_main.c +5 -3

index 9508b6c38003..652ec8a9c8a4 100644
@@ -228,6 +228,10 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
 	u32 ethr_len = skb_mac_header_len(skb);
 	u32 bufflen = ethr_len + network_len;
 
+	if (skb->protocol != htons(ETH_P_IP) &&
+	    skb->protocol != htons(ETH_P_IPV6))
+		return false;
+
 	if (skb_cow_head(skb, PSP_ENCAP_HLEN))
 		return false;
 

@@ -243,11 +247,9 @@ bool psp_dev_encapsulate(struct net *net, struct sk_buff *skb, __be32 spi,
 		ip_hdr(skb)->check = 0;
 		ip_hdr(skb)->check =
 			ip_fast_csum((u8 *)ip_hdr(skb), ip_hdr(skb)->ihl);
-	} else if (skb->protocol == htons(ETH_P_IPV6)) {
+	} else {
 		ipv6_hdr(skb)->nexthdr = IPPROTO_UDP;
 		be16_add_cpu(&ipv6_hdr(skb)->payload_len, PSP_ENCAP_HLEN);
-	} else {
-		return false;
 	}
 
 	skb_set_inner_ipproto(skb, IPPROTO_TCP);

Signature · 1 line

2.54.0

In psp_dev_create(), the DEV_ADD_NTF netlink notification is sent before the device is published to the netdev via rcu_assign_pointer(). IIRC this is intentional because a single PSP device is expected to be shared with multiple netdevs. So we are trying to default to not having the netdev info. We can change it if someone complains but for now just add a comment that it's intentional.

Signed-off-by Jakub Kicinski <[email protected]>

psp_dev_rcv() is called from the NIC driver's RX completion path before the frame reaches ip_rcv_core(), so the IP header has not been validated in SW, yet. We expect that the device has done all this validation, but let's also add the SW checks, to avoid surprises.

Signed-off-by Jakub Kicinski <[email protected]>

psp_dev_rcv() is called from the NIC driver's RX completion path before the frame reaches ip_rcv_core(), so the IP header has not been validated in SW, yet. We expect that the device has done all this validation, but let's also add the SW checks, to avoid surprises.

Signed-off-by Jakub Kicinski <[email protected]>

---
 net/psp/psp_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

net/psp/psp_main.c +9 -0

index f069117c867a..524978dfb8fd 100644
@@ -300,6 +300,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 	if (proto == htons(ETH_P_IP)) {
 		struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
 
+		if (unlikely(iph->ihl < 5))
+			return -EINVAL;
+
 		is_udp = iph->protocol == IPPROTO_UDP;
 		l3_hlen = iph->ihl * 4;
 		if (l3_hlen != sizeof(struct iphdr) &&

@@ -335,6 +338,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 	if (proto == htons(ETH_P_IP)) {
 		struct iphdr *iph = (struct iphdr *)(skb->data + l2_hlen);
 
+		if (unlikely(ntohs(iph->tot_len) < l3_hlen + encap))
+			return -EINVAL;
+
 		iph->protocol = psph->nexthdr;
 		iph->tot_len = htons(ntohs(iph->tot_len) - encap);
 		iph->check = 0;

@@ -342,6 +348,9 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
 	} else {
 		struct ipv6hdr *ipv6h = (struct ipv6hdr *)(skb->data + l2_hlen);
 
+		if (unlikely(ntohs(ipv6h->payload_len) < encap))
+			return -EINVAL;
+
 		ipv6h->nexthdr = psph->nexthdr;
 		ipv6h->payload_len = htons(ntohs(ipv6h->payload_len) - encap);
 	}

Signature · 1 line

2.54.0

Quoted 5 lines · Jakub Kicinski

psp_dev_rcv() is called from the NIC driver's RX completion path before the frame reaches ip_rcv_core(), so the IP header has not been validated in SW, yet. We expect that the device has done all this validation, but let's also add the SW checks, to avoid surprises.

If devices are expected to have verified this, should these be more noisy checks, similar to netdev_rx_csum_fault?

Quoted 9 lines · On Tue, 28 Apr 2026 20:22:34 -0400 Willem de Bruijn wrote:

Quoted 5 lines · Jakub Kicinski

psp_dev_rcv() is called from the NIC driver's RX completion path before the frame reaches ip_rcv_core(), so the IP header has not been validated in SW, yet. We expect that the device has done all this validation, but let's also add the SW checks, to avoid surprises.

If devices are expected to have verified this, should these be more noisy checks, similar to netdev_rx_csum_fault?

Maybe "expect" is a bit of a strong word, I meant "anticipate" / "suspect". Dropping invalid packet in SW doesn't seem like a huge problem, other paths in this function already do. For rx csum the problem is that we got a incorrectly math'ed out value for what is likely a valid packet.

That's just to explain my thinking, if you prefer we warn / dump skb I can respin.