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;
Date:   Wed, 22 Jul 2020 08:59:05 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Elena Petrova <lenaptr@google.com>
Cc:     linux-crypto@vger.kernel.org,
        Stephan =?iso-8859-1?Q?M=FCller?= <smueller@chronox.de>,
        Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG
 interface
Message-ID: <20200722155905.GA789@sol.localdomain>
References: <CABvBcwY44BPa+TaDwxWaEogpg3Kdkq8o9cR5gSqNGF-o6d3jrw@mail.gmail.com>
 <20200716164028.1805047-1-lenaptr@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200716164028.1805047-1-lenaptr@google.com>
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 16, 2020 at 05:40:28PM +0100, Elena Petrova wrote:
> Extending the userspace RNG interface:
>   1. adding ALG_SET_DRBG_ENTROPY setsockopt option for entropy input;
>   2. using sendmsg syscall for specifying the additional data.
> 
> Signed-off-by: Elena Petrova <lenaptr@google.com>

Can you add more details to the commit message?  E.g. why this is needed.

Also please use imperative tense, e.g. "Extend the userspace RNG interface".

>  static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  		       int flags)
>  {
> @@ -65,6 +80,7 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  	int genlen = 0;
>  	u8 result[MAXSIZE];
>  
> +	lock_sock(sock->sk);
>  	if (len == 0)
>  		return 0;

This returns without unlocking the socket.

>  	if (len > MAXSIZE)
> @@ -82,16 +98,45 @@ static int rng_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>  	 * seeding as they automatically seed. The X9.31 DRNG will return
>  	 * an error if it was not seeded properly.
>  	 */
> -	genlen = crypto_rng_get_bytes(ctx->drng, result, len);
> +	genlen = crypto_rng_generate(ctx->drng, ctx->addtl, ctx->addtl_len,
> +				     result, len);
>  	if (genlen < 0)
>  		return genlen;

Likewise.

>  
>  	err = memcpy_to_msg(msg, result, len);
>  	memzero_explicit(result, len);
> +	rng_reset_addtl(ctx);
> +	release_sock(sock->sk);
>  
>  	return err ? err : len;
>  }
>  
> +static int rng_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +	int err;
> +	struct alg_sock *ask = alg_sk(sock->sk);
> +	struct rng_ctx *ctx = ask->private;
> +
> +	lock_sock(sock->sk);
> +	if (len > MAXSIZE)
> +		len = MAXSIZE;
> +
> +	rng_reset_addtl(ctx);
> +	ctx->addtl = kmalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;

Likewise.

> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		rng_reset_addtl(ctx);
> +		return err;

Likewise.