Re: [PATCH net v2 1/4] ipv4: validate ip_options length in __ip_options_echo() against skb tail

[Ido Schimmel <idosch@nvidia.com>]

On Sun, May 24, 2026 at 12:14:35PM +0800, Qi Tang wrote:
> __ip_options_echo() re-reads each option length byte (RR/TS/SRR/CIPSO)
> from skb->data when building the echoed options into a 40-byte
> __data[] buffer.  __ip_options_compile() saved only the option offset
> into IPCB(skb)->opt, not the length.  An nftables LOCAL_IN payload
> write reachable from an unprivileged user namespace can mutate the
> length byte between parse and recvmsg, turning a parse-time validated
> 7-byte option into a 255-byte read.
> 
>   unsigned char optbuf[sizeof(struct ip_options) + 40];
>   /* in __ip_options_echo: */
>   optlen = sptr[sopt->rr + 1];        /* re-read; nft can mutate */
>   memcpy(dptr, sptr + sopt->rr, optlen); /* into 40-byte buffer */
> 
> The destination is a stack buffer in ip_cmsg_recv_retopts() and a
> DEFINE_RAW_FLEX() buffer in icmp.c / ip_output.c sized
> IP_OPTIONS_DATA_FIXED_SIZE (40).  KASAN reports a stack-out-of-bounds
> write of size 255:
> 
>   BUG: KASAN: stack-out-of-bounds in __ip_options_echo+0x7fc/0x1310
>   Write of size 255 at addr ffff88800a657950
>    __asan_memcpy+0x3c/0x60
>    __ip_options_echo+0x7fc/0x1310
>    ip_cmsg_recv_offset+0x58b/0xd10
>    udp_recvmsg+0x8da/0xc20
>    ____sys_recvmsg+0x1b1/0x620
> 
> Validate that each re-read option length stays within
> skb_tail_pointer(skb) before the memcpy.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Qi Tang <tpluszz77@gmail.com>
> Reported-by: Tong Liu <lyutoon@gmail.com>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Qi Tang <tpluszz77@gmail.com>
> ---
>  net/ipv4/ip_options.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
> index be8815ce3ac24..1cc6096e6dd9d 100644
> --- a/net/ipv4/ip_options.c
> +++ b/net/ipv4/ip_options.c
> @@ -91,6 +91,8 @@ int __ip_options_echo(struct net *net, struct ip_options *dopt,
>  
>  	if (sopt->rr) {
>  		optlen  = sptr[sopt->rr+1];
> +		if (sptr + sopt->rr + optlen > skb_tail_pointer(skb))
> +			return -EINVAL;

Both Sashiko instances flag valid issues. Please go over them. The most
obvious issues are:

1. This check only avoids reading past the skb's linear buffer. The
memcpy() below can still overflow the destination buffer which is only
40 bytes.

2. There is no validation against the original IP options length
(sopt->optlen), so we might be echoing bytes from the skb payload (past
the IP options).

>  		soffset = sptr[sopt->rr+2];
>  		dopt->rr = dopt->optlen + sizeof(struct iphdr);
>  		memcpy(dptr, sptr+sopt->rr, optlen);
> @@ -105,6 +107,8 @@ int __ip_options_echo(struct net *net, struct ip_options *dopt,
>  	}
>  	if (sopt->ts) {
>  		optlen = sptr[sopt->ts+1];
> +		if (sptr + sopt->ts + optlen > skb_tail_pointer(skb))
> +			return -EINVAL;
>  		soffset = sptr[sopt->ts+2];
>  		dopt->ts = dopt->optlen + sizeof(struct iphdr);
>  		memcpy(dptr, sptr+sopt->ts, optlen);
> @@ -145,6 +149,8 @@ int __ip_options_echo(struct net *net, struct ip_options *dopt,
>  		__be32 faddr;
>  
>  		optlen  = start[1];
> +		if (start + optlen > skb_tail_pointer(skb))
> +			return -EINVAL;
>  		soffset = start[2];
>  		doffset = 0;
>  		if (soffset > optlen)
> @@ -174,6 +180,8 @@ int __ip_options_echo(struct net *net, struct ip_options *dopt,
>  	}
>  	if (sopt->cipso) {
>  		optlen  = sptr[sopt->cipso+1];
> +		if (sptr + sopt->cipso + optlen > skb_tail_pointer(skb))
> +			return -EINVAL;
>  		dopt->cipso = dopt->optlen+sizeof(struct iphdr);
>  		memcpy(dptr, sptr+sopt->cipso, optlen);
>  		dptr += optlen;
> -- 
> 2.47.3
>