All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Ren Wei <enjou1224z@gmail.com>
Cc: netdev@vger.kernel.org, dsahern@kernel.org, davem@davemloft.net,
	edumazet@google.com, pabeni@redhat.com, horms@kernel.org,
	vega@nebusec.ai, zihanx@nebusec.ai
Subject: Re: [PATCH net 1/1] ipv4: Fix fib_nlmsg_size() for RTA_VIA nexthops
Date: Thu, 30 Jul 2026 14:06:24 +0300	[thread overview]
Message-ID: <20260730110624.GA1683700@shredder> (raw)
In-Reply-To: <3dac596cfe0ad1559c1bd1314dd4777dbcf22ee2.1785058094.git.zihanx@nebusec.ai>

On Tue, Jul 28, 2026 at 02:14:20AM +0800, Ren Wei wrote:
> From: Zihan Xi <zihanx@nebusec.ai>
> 
> fib_nlmsg_size() still estimates nexthop space as if every gateway is
> encoded as an IPv4 RTA_GATEWAY attribute.  IPv4 routes can also carry an
> IPv6 gateway, which fib_nexthop_info() dumps as RTA_VIA.  RTA_VIA needs
> more space than RTA_GATEWAY, and multipath dumps also store each
> struct rtnexthop with nla_reserve_nohdr(), not as a standalone nlattr.
> 
> As a result, a route notification can allocate an skb that is too small.
> fib_dump_info() then fails with -EMSGSIZE and rtmsg_fib() hits the
> WARN_ON() that marks such failures as a fib_nlmsg_size() bug.  With
> panic_on_warn set, this becomes a kernel panic.
> 
> Add a small helper that mirrors the per-nexthop attributes emitted by
> fib_nexthop_info(), account for RTA_VIA when the gateway family differs
> from the route family, account for the no-header rtnexthop layout used
> inside RTA_MULTIPATH, and only include RTA_FLOW when it is actually
> present.
> 
> Fixes: d15662682db2 ("ipv4: Allow ipv6 gateway with ipv4 routes")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: Codex:gpt-5.4
> Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
> Signed-off-by: Ren Wei <enjou1224z@gmail.com>
> ---
>  net/ipv4/fib_semantics.c | 71 ++++++++++++++++++++++++++++++----------
>  1 file changed, 53 insertions(+), 18 deletions(-)

Looks OK. A few nits to fix in v2 below.

> 
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 4f3c0740dde9..3b6f6cc9ffe0 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -490,6 +490,37 @@ int ip_fib_check_default(__be32 gw, struct net_device *dev)
>  	return -1;
>  }
>  
> +static size_t fib_nexthop_nlmsg_size(const struct fib_nh_common *nhc,
> +				     u8 rt_family, bool skip_oif)

The 'rt_family' argument is always AF_INET, so it can be dropped.

> +{
> +	size_t nhsize = 0;
> +
> +	switch (nhc->nhc_gw_family) {
> +	case AF_INET:
> +		nhsize += nla_total_size(4); /* RTA_GATEWAY */
> +		break;
> +	case AF_INET6:
> +		if (rt_family != nhc->nhc_gw_family)
> +			nhsize += nla_total_size(sizeof(struct rtvia) +
> +						 sizeof(struct in6_addr));
> +		else
> +			nhsize += nla_total_size(sizeof(struct in6_addr));

And the else branch is unreachable, so should be dropped as well.

> +		break;
> +	}
> +
> +	if (!skip_oif && nhc->nhc_dev)
> +		nhsize += nla_total_size(4); /* RTA_OIF */
> +
> +	if (nhc->nhc_lwtstate) {
> +		/* RTA_ENCAP_TYPE */

s/RTA_ENCAP_TYPE/RTA_ENCAP/

> +		nhsize += lwtunnel_get_encap_size(nhc->nhc_lwtstate);
> +		/* RTA_ENCAP */

s/RTA_ENCAP/RTA_ENCAP_TYPE/

> +		nhsize += nla_total_size(2);
> +	}
> +
> +	return nhsize;
> +}
> +
>  size_t fib_nlmsg_size(struct fib_info *fi)
>  {
>  	size_t payload = NLMSG_ALIGN(sizeof(struct rtmsg))
> @@ -507,32 +538,36 @@ size_t fib_nlmsg_size(struct fib_info *fi)
>  		payload += nla_total_size(4); /* RTA_NH_ID */
>  
>  	if (nhs) {
> -		size_t nh_encapsize = 0;
> -		/* Also handles the special case nhs == 1 */
> -
> -		/* each nexthop is packed in an attribute */
> -		size_t nhsize = nla_total_size(sizeof(struct rtnexthop));
> +		size_t mpsize = 0;
>  		unsigned int i;
>  
> -		/* may contain flow and gateway attribute */
> -		nhsize += 2 * nla_total_size(4);
> -
> -		/* grab encap info */
>  		for (i = 0; i < fib_info_num_path(fi); i++) {
>  			struct fib_nh_common *nhc = fib_info_nhc(fi, i);
> +			size_t nhsize;
> +
> +			nhsize = fib_nexthop_nlmsg_size(nhc, AF_INET,
> +							nhs != 1);
>  
> -			if (nhc->nhc_lwtstate) {
> -				/* RTA_ENCAP_TYPE */
> -				nh_encapsize += lwtunnel_get_encap_size(
> -						nhc->nhc_lwtstate);
> -				/* RTA_ENCAP */
> -				nh_encapsize +=  nla_total_size(2);
> +			if (nhs != 1)
> +				nhsize += sizeof(struct rtnexthop);

Should be NLA_ALIGN(sizeof(struct rtnexthop)) instead of
sizeof(struct rtnexthop), to match nla_reserve_nohdr() in
fib_add_nexthop().

      reply	other threads:[~2026-07-30 11:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27 18:14 [PATCH net 0/1] ipv4: Fix fib_nlmsg_size() for RTA_VIA nexthops Ren Wei
2026-07-27 18:14 ` [PATCH net 1/1] " Ren Wei
2026-07-30 11:06   ` Ido Schimmel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260730110624.GA1683700@shredder \
    --to=idosch@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=enjou1224z@gmail.com \
    --cc=horms@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=vega@nebusec.ai \
    --cc=zihanx@nebusec.ai \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.