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: <20200813193239.GA2470@sol.localdomain> <20200821042443.GA25695@gondor.apana.org.au>
In-Reply-To: <20200821042443.GA25695@gondor.apana.org.au>
From:   Elena Petrova <lenaptr@google.com>
Date:   Tue, 8 Sep 2020 18:23:25 +0100
Message-ID: <CABvBcwYqeGw-9oGngEm01dAt8JLxkKCLFFn9409efcLPTsJg+w@mail.gmail.com>
Subject: Re: [PATCH v5] crypto: af_alg - add extra parameters for DRBG interface
To:     Herbert Xu <herbert@gondor.apana.org.au>
Cc:     Eric Biggers <ebiggers@kernel.org>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.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

On Fri, 21 Aug 2020 at 05:24, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Since proto_ops are almost identical, and only one is used in a given kernel
> > build, why not just do:
> >
> > static struct proto_ops algif_rng_ops = {
> >        ...
> > #ifdef CONFIG_CRYPTO_USER_API_RNG_CAVP
> >        .sendmsg        = rng_sendmsg,
> > #else
> >        .sendmsg        = sock_no_sendmsg,
> > #endif
> >        ...
> > };
> >
> > Similarly for .recvmsg(), although I don't understand what's wrong with just
> > adding the lock_sock() instead...  The RNG algorithms do locking anyway, so it's
> > not like that would regress the ability to recvmsg() in parallel.  Also,
> > conditional locking depending on the kernel config makes it more difficult to
> > find kernel bugs like deadlocks.
>
> I want this to have minimal impact on anyone who's not using it.
> After all, this is something that only Google is asking for.
>
> Anyway, I wasn't looking for a compile-time ops switch, but a
> run-time one.
>
> I think what we can do is move the new newsock->ops assignment
> in af_alg_accept up above the type->accept call which would then
> allow it to be overridden by the accept call.
>
> After that you could just change newsock->ops depending on whether
> pctx->entropy is NULL or not in rng_accept_parent.

Ack, done in v6.

> As for the proto_ops duplication I don't think it's that big a
> deal, but if you're really bothered just create a macro for the
> identical bits in the struct.

I didn't create a macro to avoid complicating the code.

Thanks,
Elena