linux-kernel - Re: [PATCH] crypto: testmgr - fix RNG performance in fuzz tests

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]

Date:   Mon, 27 Feb 2023 10:19:55 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Yann Droneaud <ydroneaud@...eya.com>
Cc:     linux-crypto@...r.kernel.org,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "Jason A . Donenfeld" <Jason@...c4.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] crypto: testmgr - fix RNG performance in fuzz tests

On Mon, Feb 27, 2023 at 08:13:41AM +0000, Yann Droneaud wrote:
> > +/*
> > + * The fuzz tests use prandom instead of the normal Linux RNG since they don't
> > + * need cryptographically secure random numbers. This greatly improves the
> > + * performance of these tests, especially if they are run before the Linux RNG
> > + * has been initialized or if they are run on a lockdep-enabled kernel.
> > + */
> > +
> > +static inline void init_rnd_state(struct rnd_state *rng)
> > +{
> > + static atomic64_t next_seed;
> > +
> > + prandom_seed_state(rng, atomic64_inc_return(&next_seed));
> 
> Isn't making this deterministic defeating the purpose of fuzzing ?
> 
> Regards.

No, it's still fuzzing.  It just means that you'll no longer get different test
cases each individual time you boot the kernel.  Which seems like mostly a good
thing, since it makes test failures reproducible.

This seemed like the obvious thing to do now that we're not using an API that
doesn't provide this option.  Though now that you draw attention to it, there is
indeed a tradeoff that rare bugs will be harder to find.  I suppose that making
it deterministic ought to be a separate patch so that this one isn't doing two
different things.  I'll make this patch use get_random_u64() here instead.

- Eric