Re: [PATCH v2 net-next 10/14] ipv6: Factorise ip6_route_multipath_add().

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Simon Horman <horms@kernel.org>
Date: Fri, 11 Apr 2025 11:34:04 +0100
> > +static int ip6_route_mpath_info_create_nh(struct list_head *rt6_nh_list,
> > +					  struct netlink_ext_ack *extack)
> > +{
> > +	struct rt6_nh *nh, *nh_next, *nh_tmp;
> > +	LIST_HEAD(tmp);
> > +	int err;
> > +
> > +	list_for_each_entry_safe(nh, nh_next, rt6_nh_list, next) {
> > +		struct fib6_info *rt = nh->fib6_info;
> > +
> > +		err = ip6_route_info_create_nh(rt, &nh->r_cfg, extack);
> > +		if (err) {
> > +			nh->fib6_info = NULL;
> > +			goto err;
> > +		}
> > +
> > +		rt->fib6_nh->fib_nh_weight = nh->weight;
> > +
> > +		list_move_tail(&nh->next, &tmp);
> > +
> > +		list_for_each_entry(nh_tmp, rt6_nh_list, next) {
> > +			/* check if fib6_info already exists */
> > +			if (rt6_duplicate_nexthop(nh_tmp->fib6_info, rt)) {
> > +				err = -EEXIST;
> > +				goto err;
> > +			}
> > +		}
> > +	}
> > +out:
> > +	list_splice(&tmp, rt6_nh_list);
> > +	return err;
> 
> Hi Kuniyuki-san,
> 
> Perhaps it can't happen in practice,

Yes, it never happens by patch 1 as rtm_to_fib6_multipath_config()
returns an error in such a case.


> but if the loop above iterates zero
> times then err will be used uninitialised. As it's expected that err is 0
> here, perhaps it would be simplest to just:
> 
> 	return 0;

If we want to return 0 above, we need to duplicate list_splice() at
err: and return err; there.  Or initialise err = 0, but this looks
worse to me.

Btw, was this caught by Smatch, Coverity, or something ?  I don't
see such a report at CI.
https://patchwork.kernel.org/project/netdevbpf/patch/20250409011243.26195-11-kuniyu@amazon.com/

If so, I'm just curious if we have an official guideline for
false-positives flagged by such tools, like we should care about it
while writing a code and should try to be safer to make it happy.

We are also running Coverity for the mainline kernel and have tons
of false-positive reports due to lack of contexts.

Thanks!

> 
> > +err:
> > +	ip6_route_mpath_info_cleanup(rt6_nh_list);
> > +	goto out;