Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200729154501.2461888-1-lenaptr@google.com> <20200731072338.GA17285@gondor.apana.org.au>
In-Reply-To: <20200731072338.GA17285@gondor.apana.org.au>
From:   Elena Petrova <lenaptr@google.com>
Date:   Mon, 3 Aug 2020 15:48:02 +0100
Message-ID: <CABvBcwY-F6Euo2SAY6MKpT0KP7OtyswLhUmShPNPfB0qqL6heQ@mail.gmail.com>
Subject: Re: [PATCH v4] crypto: af_alg - add extra parameters for DRBG interface
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>, Eric Biggers <ebiggers@kernel.org>,
        Stephan Mueller <smueller@chronox.de>,
        Ard Biesheuvel <ardb@kernel.org>,
        Jeffrey Vander Stoep <jeffv@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

Hi Herbert,

> > +#ifdef CONFIG_CRYPTO_USER_API_CAVP_DRBG
> > +static int rng_setentropy(void *private, const u8 *entropy, unsigned int len)
>
> Please use __maybe_unused instead of the ifdef.

Ack

On Fri, 31 Jul 2020 at 08:27, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > lock_sock() would solve the former.  I'm not sure what should be done about
> > rng_recvmsg().  It apparently relies on the crypto_rng doing its own locking,
> > but maybe it should just use lock_sock() too.
>
> The lock_sock is only needed if you're doing testing.  What I'd
> prefer is to have a completely different code-path for testing.

sendmsg is used for "Additional Data" input, and unlike entropy, it
could be useful outside of testing. But if you confirm it's not
useful, then yes, I can decouple the testing parts.

> How about you fork the code in rng_accept_parent so that you have
> a separate proto_ops for the test path that is used only if
> setentropy has been called? That way all of this code could
> magically go away if the CONFIG option wasn't set.

Depends on the comment above, but otherwise, my only concern is that
the testing variant of rng_recvmsg would be largely copy-pasted from
the normal rng_recvmsg, apart from a few lines of lock/release and
crypto_rng_generate/crypto_rng_get_bytes.


Thanks!
Elena