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:   Mon, 13 Jul 2020 10:25:11 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Elena Petrova <lenaptr@google.com>
Cc:     linux-crypto@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH 1/1] crypto: af_alg - add extra parameters for DRBG
 interface
Message-ID: <20200713172511.GB722906@gmail.com>
References: <20200713164857.1031117-1-lenaptr@google.com>
 <20200713164857.1031117-2-lenaptr@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200713164857.1031117-2-lenaptr@google.com>
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 13, 2020 at 05:48:57PM +0100, Elena Petrova wrote:
> +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;
> +
> +	reset_addtl(ctx);
> +	ctx->addtl = kzalloc(len, GFP_KERNEL);
> +	if (!ctx->addtl)
> +		return -ENOMEM;
> +
> +	err = memcpy_from_msg(ctx->addtl, msg, len);
> +	if (err) {
> +		reset_addtl(ctx);
> +		return err;
> +	}
> +	ctx->addtl_len = len;
> +
> +	return 0;
> +}

This is also missing any sort of locking, both between concurrent calls to
rng_sendmsg(), and between rng_sendmsg() and rng_recvmsg().

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.

- Eric