Re: [PATCH] ndp: Suppress Coverity false positive for random()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2644 bytes --]

On Thu, May 14, 2026 at 01:08:26AM +0200, Stefano Brivio wrote:
> On Wed, 13 May 2026 12:26:17 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
> > Coverity flags the random() call in ndp_timer() with the dont_call
> > checker, warning that it should not be used for security-related
> > applications.
> > 
> > This is a false positive: random() is used here to jitter the interval
> > between unsolicited Router Advertisements as required by RFC 4861, to
> > prevent synchronisation between routers on a link.  No cryptographic
> > strength is needed.
> > 
> > Suppress the warning with an inline Coverity annotation.
> > 
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > ---
> >  ndp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/ndp.c b/ndp.c
> > index 1f2bcb0cc7ea..614932ac5829 100644
> > --- a/ndp.c
> > +++ b/ndp.c
> > @@ -441,6 +441,7 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
> >  	 * again, it's close enough for our purposes.
> >  	 */
> >  	interval = min_rtr_adv_interval +
> > +		/* coverity[dont_call:FALSE] */
> 
> Sorry, I should have mentioned this to you explicitly, but we discussed
> this in the past and we decided against having explicit suppressions
> for warnings from Coverity Scan (at least, that would be my strong
> preference).
> 
> The reason is that I would like to avoid referring to trademarks as
> much as possible, as they might raise "interesting" legal questions,
> and at the same time we have very little control or visibility into how
> these suppressions evolve in future versions of the checker.

If this suppression format also works for the sort-of-public
scan.coverity.com, that mitigates those later concerns somewhat.
Still doesn't remove the kind of implicit endorsement of including a
trademark, which I can understand why we'd want to avoid.

> In this case, by the way, despite the fact that we use this to add some
> randomness to the timing of router advertisements as required by RFC
> 4861, I started wondering recently if an attacker (I'm mostly thinking
> about denials of service) could actually gain anything from making
> these intervals predictable.
> 
> If that's the case, perhaps we could just switch to getrandom() and
> be done with it.

Hm, maybe, yes.  These announcements are infrequent enuogh that the
extra cost of getrandom() shouldn't really be a problem.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]