Skip to contentSkip to conversation

xfrm4: use a full socket for local errors

PATCH Applied Fourie Zhang 2 participants 2 messages Oct 22 – Oct 30

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 xfrm4: use a full socket for local errors

From: Fourie Zhang <[email protected]>

xfrm4_local_error() passes skb->sk to inet_sk() and ip_local_error() without ensuring it points to a full socket. TCP SYN-ACK skbs can be owned by TCP_NEW_SYN_RECV request sockets.

xfrm4_tunnel_check_size() already checks sk_fullsock() since commit 1620c88887b1 ("xfrm: Fix the usage of skb->sk"), so iptfs_is_too_big() is the only remaining IPv4 caller passing a request socket here: the sk it tests is the listener, while skb->sk is the request socket.

When IP-TFS rejects such an skb because it exceeds its payload MTU, the local error path treats the request socket as a struct inet_sock. KASAN reports a slab-out-of-bounds read in ip_local_error() beyond the request_sock_TCP allocation.

Resolve the skb owner with skb_to_full_sk() before accessing inet_sock state or reporting the error. This maps a request socket to its listener and rejects sockets that cannot be used as full sockets, matching what commit 6973a21ee73c ("ipv6: xfrm: use full sockets in local error paths") did for xfrm6_local_error().

Fixes 0e4fbf013fa5 ("xfrm: iptfs: add user packet (tunnel ingress) handling")
Assisted-by tencentos-corvus-ai:hy4-preview

---
v2 (Eric Dumazet):
- Fixes: use the IP-TFS commit
- Note why IP-TFS is the only remaining IPv4 caller
- Reverse xmas tree; add Reviewed-by

v1: https://lore.kernel.org/netdev/[email protected]/

A KASAN reproducer for this issue is available if requested.

 net/ipv4/xfrm4_output.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
net/ipv4/xfrm4_output.c +5 -2
index 0ae67d537499..fffa3c4f456e 100644
@@ -38,9 +38,12 @@ int xfrm4_output(struct net *net, struct sock *sk, struct sk_buff *skb)
 
 void xfrm4_local_error(struct sk_buff *skb, u32 mtu)
 {
+	struct sock *sk = skb_to_full_sk(skb);
 	struct iphdr *hdr;
 
+	if (!sk)
+		return;
+
 	hdr = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb);
-	ip_local_error(skb->sk, EMSGSIZE, hdr->daddr,
-		       inet_sk(skb->sk)->inet_dport, mtu);
+	ip_local_error(sk, EMSGSIZE, hdr->daddr, inet_sk(sk)->inet_dport, mtu);
 }
Signature · 1 line
2.43.7
maintainer Applied
#1 lore raw
Quoted 22 lines · Fourie Zhang

From: Fourie Zhang <[email protected]>

xfrm4_local_error() passes skb->sk to inet_sk() and ip_local_error() without ensuring it points to a full socket. TCP SYN-ACK skbs can be owned by TCP_NEW_SYN_RECV request sockets.

xfrm4_tunnel_check_size() already checks sk_fullsock() since commit 1620c88887b1 ("xfrm: Fix the usage of skb->sk"), so iptfs_is_too_big() is the only remaining IPv4 caller passing a request socket here: the sk it tests is the listener, while skb->sk is the request socket.

When IP-TFS rejects such an skb because it exceeds its payload MTU, the local error path treats the request socket as a struct inet_sock. KASAN reports a slab-out-of-bounds read in ip_local_error() beyond the request_sock_TCP allocation.

Resolve the skb owner with skb_to_full_sk() before accessing inet_sock state or reporting the error. This maps a request socket to its listener and rejects sockets that cannot be used as full sockets, matching what commit 6973a21ee73c ("ipv6: xfrm: use full sockets in local error paths") did for xfrm6_local_error().

Quoted 7 lines
Fixes 0e4fbf013fa5 ("xfrm: iptfs: add user packet (tunnel ingress) handling")
Assisted-by tencentos-corvus-ai:hy4-preview

Applied, thanks a lot!