Hi Stephan,
On Tue, 21 Jul 2020 at 14:19, Stephan Mueller <smueller@xxxxxxxxxx> wrote:
>
> Am Dienstag, 21. Juli 2020, 14:55:14 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > > > +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> > > > +static int rng_setentropy(void *private, const u8 *entropy, unsigned
> > > > int
> > > > len) +{
> > > > + struct rng_parent_ctx *pctx = private;
> > > > + u8 *kentropy = NULL;
> > > > +
> > > > + if (!capable(CAP_SYS_ADMIN))
> > > > + return -EPERM;
> > > > +
> > > > + if (pctx->entropy)
> > > > + return -EINVAL;
> > > > +
> > > > + if (len > MAXSIZE)
> > > > + len = MAXSIZE;
> > > > +
> > > > + if (len) {
> > > > + kentropy = memdup_user(entropy, len);
> > > > + if (IS_ERR(kentropy))
> > > > + return PTR_ERR(kentropy);
> > > > + }
> > > > +
> > > > + crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > > > + pctx->entropy = kentropy;
> > >
> > > Why do you need to keep kentropy around? For the check above whether
> > > entropy was set, wouldn't a boolean suffice?
> >
> > I need to keep the pointer to free it after use. Unlike the setting of
> > the key, DRBG saves the entropy pointer in one of its internal
> > structures, but doesn't do any memory
> > management. I had only two ideas on how to prevent memory leaks:
> > either change drbg code to deal with the memory, or save the pointer
> > somewhere inside the socket. I opted for the latter. But if you know a
> > better approach I'm happy to rework my code accordingly.
>
> I was thinking of calling crypto_rng_alg(pctx->drng)->seed() directly after
> set_ent. This call performs a DRBG instantatiate where the entropy buffer is
> used. See crypto_drbg_reset_test for the approach.
>
> But maybe you are right, the test "entropy" buffer inside the DRBG currently
> cannot be reset. So, for sanity purposes, you need to keep it around.
I looked into this, and afaik `->seed()` needs the seed buffer (a.k.a.
key); and seed() is also invoked on ALG_SET_KEY setsockopt. So we
would need both entropy and seed values at the same time. To avoid
complicating the matters, I decided to leave the code as is. I added a
comment in v3 [https://lore.kernel.org/linux-crypto/20200728155159.2156480-1-lenaptr@xxxxxxxxxx/]
explaining why the `kentropy` pointer is saved.
> Ciao
> Stephan
Regards,
Elena
