Re: [PATCH 4/4] ndp: Use high quality entropy in NDP timer even if not needed

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 16 Jul 2026 15:45:30 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Jul 16, 2026 at 01:25:23AM +0200, Stefano Brivio wrote:
> > ...instead of calling random(), to make static checkers happy.
> > 
> > I don't think that an attacker could actually gain anything by making
> > router advertisement intervals predictable, but a doubt remains, and
> > this is cheap enough that we might just want to do this to get rid of
> > the noise from static checkers informing us that random() shouldn't be
> > used.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  ndp.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/ndp.c b/ndp.c
> > index 1f2bcb0..43457b3 100644
> > --- a/ndp.c
> > +++ b/ndp.c
> > @@ -413,6 +413,7 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
> >  {
> >  	time_t max_rtr_adv_interval = DEFAULT_MAX_RTR_ADV_INTERVAL;
> >  	time_t min_rtr_adv_interval, interval;
> > +	long random_part;
> >  
> >  	if (!tap_is_ready(c) || c->no_ra || now->tv_sec < next_ra)
> >  		return;
> > @@ -433,15 +434,17 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
> >  	 * and causing flurries of RAs at the same time.
> >  	 *
> >  	 * This random doesn't need to be cryptographically strong, so random(3)
> > -	 * is fine.  Other routers on the link also want to avoid
> > -	 * synchronisation, and anything malicious has much easier ways to cause
> > -	 * trouble.
> > +	 * would be fine.  Other routers on the link also want to avoid
> > +	 * synchronisation, and anything malicious would have much easier ways
> > +	 * to cause trouble.  However, for the sake of static checkers, use high
> > +	 * quality entropy as provided by raw_random().
> >  	 *
> >  	 * The modulus also makes this not strictly a uniform distribution, but,
> >  	 * again, it's close enough for our purposes.
> >  	 */
> > +	raw_random(&random_part, sizeof(random_part));
> >  	interval = min_rtr_adv_interval +
> > -		random() % (max_rtr_adv_interval - min_rtr_adv_interval);
> > +		   random_part % (max_rtr_adv_interval - min_rtr_adv_interval);  
> 
> Although it returns a signed long, random() is explicitly defined to
> only return values between 0 and 2^31-1.

Oops, I missed that part. I would have naturally used uint32_t here but
then I looked (too quickly) at the prototype of random() and concluded
it would be better to make it equivalent... except it's not.

> Using raw_random() means we
> can get anything in the full range of a 'long', including negative
> numbers.  Is that going to mess up our calculations here?

I don't think in any catastrophic way, but it might, yes.

> Might be safer to make random_part a uint32_t, then cast it to a
> time_t for the arithmetic.

Right, v2 does that.

-- 
Stefano