netdev - Re: [PATCH RESEND v2 net-next 01/14] ipv6: Validate RTA_GATEWAY of RTA_MULTIPATH in rtm_to_fib6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <37b0cc42-69ea-42bf-b4d8-304cba0d2dd3@redhat.com>
Date: Wed, 16 Apr 2025 10:22:33 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
 "David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Simon Horman <horms@...nel.org>, Kuniyuki Iwashima <kuni1840@...il.com>,
 netdev@...r.kernel.org
Subject: Re: [PATCH RESEND v2 net-next 01/14] ipv6: Validate RTA_GATEWAY of
 RTA_MULTIPATH in rtm_to_fib6_config().

On 4/14/25 8:14 PM, Kuniyuki Iwashima wrote:
> We will perform RTM_NEWROUTE and RTM_DELROUTE under RCU, and then
> we want to perform some validation out of the RCU scope.
> 
> When creating / removing an IPv6 route with RTA_MULTIPATH,
> inet6_rtm_newroute() / inet6_rtm_delroute() validates RTA_GATEWAY
> in each multipath entry.
> 
> Let's do that in rtm_to_fib6_config().
> 
> Note that now RTM_DELROUTE returns an error for RTA_MULTIPATH with
> 0 entries, which was accepted but should result in -EINVAL as
> RTM_NEWROUTE.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
>  net/ipv6/route.c | 82 +++++++++++++++++++++++++-----------------------
>  1 file changed, 43 insertions(+), 39 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 210b84cecc24..237e31f64a4a 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -5050,6 +5050,44 @@ static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
>  	[RTA_FLOWLABEL]		= { .type = NLA_BE32 },
>  };
>  
> +static int rtm_to_fib6_multipath_config(struct fib6_config *cfg,
> +					struct netlink_ext_ack *extack)
> +{
> +	struct rtnexthop *rtnh;
> +	int remaining;
> +
> +	remaining = cfg->fc_mp_len;
> +	rtnh = (struct rtnexthop *)cfg->fc_mp;
> +
> +	if (!rtnh_ok(rtnh, remaining)) {
> +		NL_SET_ERR_MSG(extack, "Invalid nexthop configuration - no valid nexthops");
> +		return -EINVAL;
> +	}
> +
> +	do {

I think the initial checks and the loop could be rewritten reducing the
indentation and calling the helper only once with something alike:

	for (i = 0; rtnh_ok(rtnh, remaining);
	     i++, rtnh = rtnh_next(rtnh, &remaining)) {
		int attrlen = rtnh_attrlen(rtnh);
		if (!attrlen)
			continue;

		// ...
	}
	if (i == 0) {
		NL_SET_ERR_MSG(extack, "Invalid nexthop configuration - no valid
nexthops");
		// ..

I guess it's a bit subjective, don't repost just for this.

/P