Re: [PATCH net-next] tls: check return value of strp_load_anchor_with_queue

[Geliang Tang <geliang@kernel.org>]

Hi Paolo,

mptcp-list only.

On Fri, 2025-11-28 at 16:01 +0100, Paolo Abeni wrote:
> On 11/28/25 1:55 PM, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > In tls_strp_load_anchor_with_queue(), when first is null, strp-
> > >anchor is
> > not successfully initialized. Accessing strp->anchor afterward will
> > result
> > in a memory access error (for example, BUG: KASAN: slab-use-after-
> > free in
> > skb_copy_bits).
> 
> tls_strp_load_anchor_with_queue() has:
> 
> 	WARN_ON_ONCE(!first)

I printed sk_state of strp->sk when first was null, and found it to be
8, which corresponds to TCP_CLOSE_WAIT.

Following further debugging, it was found that the issue lies in
tls_strp_read_sock() in [1] ("tls: add MPTCP protocol support"), where
I used mptcp_inq_hint() to replace tcp_inq() as follows:

-	inq = tcp_inq(strp->sk);
+	inq = strp->sk->sk_protocol == IPPROTO_MPTCP ?
+	      mptcp_inq_hint(strp->sk) :
+	      tcp_inq(strp->sk);
 	if (inq < 1)
 		return 0;

However, mptcp_inq_hint() returns 1 when sk->sk_state == TCP_CLOSE or
sk->sk_shutdown & RCV_SHUTDOWN, which is inconsistent with the behavior
of tcp_inq().

I can modify the condition here from 'if (inq < 1)' to 'if (inq <= 1)'
to fix the issue, but I know this isn't a good solution. I need to
reimplement an mptcp_inq() helper instead of directly using
mptcp_inq_hint(), right? Could you give me some advice?

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/patch/2a0f438fbd4c5ddf7d8153bfc8aa44cfefa58c45.1763800601.git.tanggeliang@kylinos.cn/

> 
> and AFAICS all the tls_strp_load_anchor_with_queue() call sites
> ensure
> that the receive queue is not empty before invoking such function.
> 
> Hitting the above condition is a symtom of a prior issue that must be
> identified and fixed. Please try to solve such problem instead.




> 
> /P