Re: [PATCH net-next 1/6] net: lwtunnel: change encap fill order

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: lwtunnel: incomplete rollback on fallback path leaves orphan RTA_ENCAP_TYPE
--

commit b32da4e3fb34d0c2a7a1afb438a76ca410be21ec
Author: Hangbin Liu <liuhangbin@kylinos.cn>

net: lwtunnel: change encap fill order

This commit changes the order in which lwtunnel netlink attributes are
emitted. It places the encap_type_attr before the encap_attr nest to
ensure userspace YNL can decode the sub-messages correctly.

> diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
> index b01a395d9a966..f13dfa02e026e 100644
> --- a/net/core/lwtunnel.c
> +++ b/net/core/lwtunnel.c
> @@ -240,6 +240,10 @@ int lwtunnel_fill_encap(struct sk_buff *skb, struct lwtunnel_state *lwtstate,
>  	    lwtstate->type > LWTUNNEL_ENCAP_MAX)
>  		return 0;
>  
> +	ret = nla_put_u16(skb, encap_type_attr, lwtstate->type);
> +	if (ret)
> +		return ret;
> +
>  	nest = nla_nest_start_noflag(skb, encap_attr);
>  	if (!nest)
>  		return -EMSGSIZE;
> @@ -254,9 +258,6 @@ int lwtunnel_fill_encap(struct sk_buff *skb, struct lwtunnel_state *lwtstate,
>  	if (ret)
>  		goto nla_put_failure;

[Severity: Medium]
Does this leave an orphaned attribute in the buffer on the fallback path?

If ops->fill_encap is unsupported, it evaluates to -EOPNOTSUPP and execution
jumps to nla_put_failure:

net/core/lwtunnel.c:lwtunnel_fill_encap() {
...
nla_put_failure:
	nla_nest_cancel(skb, nest);

	return (ret == -EOPNOTSUPP ? 0 : ret);
}

Because nla_put_u16() was moved before nla_nest_start_noflag(),
nla_nest_cancel() only unwinds the buffer up to the start of the nest.
This leaves the encap_type_attr orphaned in the skb payload.

Since the function returns 0 (success) in the -EOPNOTSUPP case, the caller
is prevented from discarding the message, which results in a malformed
netlink message being sent to userspace.

Could we record the skb tail pointer before nla_put_u16() and restore it on
failure, or is there another preferred way to unwind the encap_type_attr?

>  	nla_nest_end(skb, nest);
> -	ret = nla_put_u16(skb, encap_type_attr, lwtstate->type);
> -	if (ret)
> -		goto nla_put_failure;
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-ynl_rt_encap-v1-0-fbbe6e680571@kylinos.cn?part=1