xfrm4: use a full socket for local errors
Subsystems (4)
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().
---
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
@@ -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
Quoted 22 lines · Fourie Zhang
From: Fourie Zhang <[email protected]>
xfrm4_local_error()passesskb->sktoinet_sk()andip_local_error()without ensuring it points to a full socket. TCP SYN-ACK skbs can be owned byTCP_NEW_SYN_RECVrequest sockets.
xfrm4_tunnel_check_size()already checkssk_fullsock()since commit1620c88887b1("xfrm: Fix the usage ofskb->sk"), soiptfs_is_too_big()is the only remaining IPv4 caller passing a request socket here: the sk it tests is the listener, whileskb->skis 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 inip_local_error()beyond therequest_sock_TCPallocation.Resolve the skb owner with
skb_to_full_sk()before accessinginet_sockstate or reporting the error. This maps a request socket to its listener and rejects sockets that cannot be used as full sockets, matching what commit6973a21ee73c("ipv6: xfrm: use full sockets in local error paths") did forxfrm6_local_error().
Quoted 7 lines
Fixes 0e4fbf013fa5 ("xfrm: iptfs: add user packet (tunnel ingress) handling")Reported-by TencentOS Corvus AI <[email protected]>Reviewed-by Eric Dumazet <[email protected]>Assisted-by tencentos-corvus-ai:hy4-previewSigned-off-by Fourie Zhang <[email protected]>
Applied, thanks a lot!