netdev - Re: [PATCH v2 net-next 10/14] ipv6: Factorise ip6_route_multipath

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250415183845.GE395307@horms.kernel.org>
Date: Tue, 15 Apr 2025 19:38:45 +0100
From: Simon Horman <horms@...nel.org>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com,
	kuba@...nel.org, kuni1840@...il.com, netdev@...r.kernel.org,
	pabeni@...hat.com
Subject: Re: [PATCH v2 net-next 10/14] ipv6: Factorise
 ip6_route_multipath_add().

On Mon, Apr 14, 2025 at 11:06:58AM -0700, Kuniyuki Iwashima wrote:
> From: Simon Horman <horms@...nel.org>
> Date: Mon, 14 Apr 2025 15:52:26 +0100
> > On Fri, Apr 11, 2025 at 12:33:46PM -0700, Kuniyuki Iwashima wrote:
> > > From: Simon Horman <horms@...nel.org>
> > > Date: Fri, 11 Apr 2025 11:34:04 +0100

...

> > > > 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.
> > 
> > Thanks. I should have dug a bit deeper to determine that this
> > is a false-positive.
> > 
> > > 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/
> > 
> > Sorry for not mentioning that it was flagged by Smatch,
> > I certainly should have done so.
> 
> Thanks for confirming!
> 
> > 
> > 
> > > 
> > > 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.
> > 
> > I think that the current non-guideline is that we don't change
> > code just to keep the tools happy. Perhaps we should add something
> > about that to the process document?
> 
> Makes sense.
> 
> But looks like the series was marked Changes Requested, not sure
> if it's accidental or intentional, so I'll resend v2 to see others'
> opinion.

I'm not sure either.
But I agree that a v2 is a good way forward.