Re: [PATCH] net: dst_metadata: fix fortify trap + overread in skb_metadata_dst_cmp

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 3 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message and the new comment in
  skb_metadata_dst_cmp() (include/net/dst_metadata.h) give the wrong…
- [Medium] The Fixes: tag points at 69050f8d6d07 ("treewide: Replace
  kmalloc with kmalloc_obj for non-scalar types", Feb 2026), but the…
- [Medium] The commit trailers are Reported-by, Closes, Cc: stable and
  'Assisted-by: LLM', with no Signed-off-by.

--

net: dst_metadata: fix fortify trap + overread in skb_metadata_dst_cmp

This commit reworks the METADATA_IP_TUNNEL case in skb_metadata_dst_cmp().
It first checks that both sides have the same options_len. It then compares
the fixed part of struct ip_tunnel_info and the options in two separate
memcmp() calls through ip_tunnel_info_opts(). The goal is to fix a
FORTIFY_SOURCE panic seen in GRO on a geneve device.

Closes: https://lore.kernel.org/netdev/20261003005449.2675.1@jpsergent.gmail.com/

> Fixes: 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for non-scalar types")

[Severity: Medium]
Is this the right Fixes target? The commit message says the crash was seen
on 6.18.54. In v6.18, metadata_dst_alloc() still allocates with
kmalloc(struct_size(md_dst, u.tun_info.options, optslen), flags), not
kmalloc_flex().

Both pieces needed for the trap are older than 69050f8d6d07:

  - the __counted_by(options_len) annotation on options[] in struct
    ip_tunnel_info, added by bb5e62f2d547 ("net: Add options as a flexible
    array to struct ip_tunnel_info")

  - the memcmp() in skb_metadata_dst_cmp() that uses a's options_len for
    both sides, which dates back to 3fcece12bc1b

This patch is Cc: stable. With the current Fixes target, would the fix miss
6.15 through 6.19, including the 6.18 tree where the crash was seen? Would
bb5e62f2d547 be a better target?

> Cc: stable@vger.kernel.org
> Reported-by: Jean-Paul Sergent <jpsergent@gmail.com>
> Closes: https://lore.kernel.org/netdev/20261003005449.2675.1@jpsergent.gmail.com/
> Assisted-by: LLM

[Severity: Medium]
The trailer block has no Signed-off-by line.
Documentation/process/coding-assistants.rst says the human submitter is
responsible for "Adding their own Signed-off-by tag to certify the DCO".

Could a respin add a Signed-off-by so the patch can be applied?

> diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
> index f45d1e3163f00..60878ea1eb50a 100644
> --- a/include/net/dst_metadata.h
> +++ b/include/net/dst_metadata.h
> @@ -115,10 +115,30 @@ static inline int skb_metadata_dst_cmp(const struct sk_buff *skb_a,
>  	case METADATA_HW_PORT_MUX:
>  		return memcmp(&a->u.port_info, &b->u.port_info,
>  			      sizeof(a->u.port_info));
> -	case METADATA_IP_TUNNEL:
> -		return memcmp(&a->u.tun_info, &b->u.tun_info,
> -			      sizeof(a->u.tun_info) +
> -					 a->u.tun_info.options_len);
> +	case METADATA_IP_TUNNEL: {
> +		int ret;
> +
> +		/* Options lengths must match, or the options memcmp below
> +		 * would read past b's allocation when b carries fewer
> +		 * options than a.
> +		 */
> +		if (a->u.tun_info.options_len != b->u.tun_info.options_len)
> +			return 1;
> +		ret = memcmp(&a->u.tun_info, &b->u.tun_info,
> +			     sizeof(a->u.tun_info));
> +		if (ret)
> +			return ret;
> +		/* Compare the options through the flex-array member so the
> +		 * compiler's __counted_by(options_len) view stays consistent
> +		 * with the read length (same shape as the tun_dst_unclone
> +		 * fix); a single memcmp of struct+options trips
> +		 * CONFIG_FORTIFY_SOURCE when options_len is still 0 from
> +		 * allocation time.
> +		 */

[Severity: Medium]
Does this comment, and the commit message, describe the actual root cause?
The commit message says:

  kmalloc_flex() in metadata_dst_alloc() sets __counted_by for the
  structure to options_len, which is then initialized to zero, so the
  compiler's view of the metadata_dst tail is 96 bytes at the time of
  the access. Geneve carries 108 bytes of options

On the geneve RX path, options_len is overwritten before the skb reaches
gro_cell_poll():

geneve_rx()
  udp_tun_rx_dst() -> ... -> tun_rx_dst()
      tun_dst->u.tun_info.options_len = 0;
  ip_tunnel_info_opts_set(&tun_dst->u.tun_info,
                          gnvh->options, gnvh->opt_len * 4, flags);
      info->options_len = len;

So by the time gro_list_prepare() calls skb_metadata_dst_cmp(p, skb), both
objects hold their real options lengths.

The old code read sizeof(a->u.tun_info) + a->u.tun_info.options_len bytes,
and FORTIFY sizes a's object with the same options_len, so a's side cannot
trip. That means "108 byte read of buffer size 96" points to
a->options_len == 12 and b->options_len == 0. In other words, 12 bytes of
options, not 108.

If so, the real bug is the old memcmp() using a's options_len for both
sides. __counted_by only exposed it. Without FORTIFY, it is a slab
out-of-bounds read past b's allocation.

The options_len pre-check, which the commit message calls a "While here"
fix, is what actually stops the panic. Once the lengths are known to be
equal, a single memcmp() of sizeof(tun_info) + options_len would also pass
FORTIFY on both sides, so the two-stage split does not seem to add
anything.

The commit message also says this was seen "under sustained cross-node RX".
Can any Geneve sender trigger it remotely by mixing packets with and
without options in the same inner flow?

Could the commit message and this comment be updated to describe the
options_len mismatch between a and b as the root cause?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261003005449.2675.2%40jpsergent.gmail.com