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

[Stefano Brivio <sbrivio@redhat.com>]

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.

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.

-- 
Stefano