Re: [PATCH mptcp-next] mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mptcp: missed MIB increment for backlog packets due to mptcp_skb_net()
--

commit 69466d93fa353450c6162f23ad6457846e513a9d
Author: T S Rameshkumar <rameshsv06@gmail.com>

mptcp: add MPTCP_MIB_RCVINVALIDOPT counter for invalid options

This commit introduces the MPTCP_MIB_RCVINVALIDOPT counter to track invalid
MPTCP options encountered during parsing. To perform the increment, it attempts
to resolve the net pointer from the socket buffer in mptcp_skb_net() by
checking skb->sk, skb->dev, and skb_dst.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/628

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index adbf8e0773dbb..a1af2acfa791b 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c

[ ... ]

> @@ -419,13 +422,28 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		break;
>  
>  	default:
> +bad_opt:
> +		if (net)
> +			MPTCP_INC_STATS(net, MPTCP_MIB_RCVINVALIDOPT);
>  		break;
>  	}
>  }
>  
> +static inline struct net *mptcp_skb_net(const struct sk_buff *skb)
> +{
> +	if (skb->sk)
> +		return sock_net(skb->sk);
> +	if (skb->dev)
> +		return dev_net_rcu(skb->dev);
> +	if (skb_dst(skb) && skb_dst_dev_rcu(skb))
> +		return skb_dst_dev_net_rcu(skb);
> +	return NULL;
> +}

[Severity: Medium]
For TCP packets processed from the socket backlog queue, is it possible for
mptcp_skb_net() to always return NULL?

When a packet is processed from the backlog (e.g. via tcp_v4_do_rcv()),
skb->dev is cleared unconditionally, skb_dst is dropped, and skb->sk is not
yet set for incoming packets.

Because all three pointers are NULL, the function will return NULL. This will
cause the MIB increment at the bad_opt label to be silently skipped for any
malformed options present in backlog packets.

Since the callers of mptcp_get_options() generally have access to the relevant
socket, could they pass the known net or sk directly instead of deriving it
from the skb?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013085604.115478-1-rameshkumar.t@phytecembedded.in?part=1