Re: [PATCH net 4/7] net: tls: fix off-by-one in sg_chain entry count for wrapped sk_msg ring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] In TLS 1.3, a wrapped scatterlist with `end == 0` creates a chain-to-chain pointer structure that the scatterlist API cannot traverse, leading to invalid `struct page` dereferences in the crypto engine.
--

commit ea91dff65607bff05b2c2e5b43839119ab72f451
Author: Jakub Kicinski <kuba@kernel.org>

net: tls: fix off-by-one in sg_chain entry count for wrapped sk_msg ring

This commit fixes the entry count passed to sg_chain when chaining a
wrapped sk_msg ring. It replaces MAX_SKB_FRAGS with NR_MSG_FRAG_IDS to
ensure the chaining pointer lands in the correct reserved slot.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index bf8f022e1895..2c6a3c4a9fcc 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
[ ... ]
>  	if (prot->version == TLS_1_3_VERSION) {
>  		/* Add content type to end of message.  No padding added */
>  		sg_set_buf(&rec->sg_content_type, &rec->content_type, 1);
>  		sg_mark_end(&rec->sg_content_type);
>  		sg_chain(msg_pl->sg.data, msg_pl->sg.end + 1,
>  			 &rec->sg_content_type);
>  	} else {
>  		sg_mark_end(sk_msg_elem(msg_pl, i));
>  	}
>  
>  	if (msg_pl->sg.end < msg_pl->sg.start) {
>  		sg_chain(&msg_pl->sg.data[msg_pl->sg.start],
> -			 MAX_SKB_FRAGS - msg_pl->sg.start + 1,
> +			 NR_MSG_FRAG_IDS - msg_pl->sg.start + 1,
>  			 msg_pl->sg.data);
>  	}

If the wrapped message perfectly fills the end of the array such that
msg_pl->sg.end is 0, does the TLS 1.3 block create a chain pointer at
msg_pl->sg.data[0]?

Because the ring wrapped, the second sg_chain call links the tail of the
array to msg_pl->sg.data, which starts at index 0.

This would create a chain where the wrap link points directly to another chain
link. The scatterlist API sg_next iterator does not recursively resolve
consecutive chain links.

When stepping past the end of the ring, sg_next would follow the wrap chain,
land on msg_pl->sg.data[0], and return that chain element directly to the
caller.

If the crypto engine then calls sg_page on this returned element, could it
interpret the chain pointer as a struct page address and panic?

While this patch fixes the ring modulus so it no longer corrupts valid payload
data, does the underlying double-chain issue still exist for TLS 1.3 records
that wrap exactly at msg_pl->sg.end == 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429222944.2139041-1-kuba@kernel.org?part=4