Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAKv+Gu-VqfFsW+nrG+-2g1-eu6S+ZuD7qaN9aTchwD=Bcj_giw@mail.gmail.com>
 <04D32F59-34D4-4EBF-80E3-69088D14C5D8@amacapital.net> <CAKv+Gu8s6AuZfdVUSmpgi-eY_9oZr-j4sdFygUOR3uvQXji+rQ@mail.gmail.com>
 <CALCETrXWSu_fY5BetMah=iEOqSgkOphMOKcMrtiWyN0QqbZspw@mail.gmail.com> <CAKv+Gu8u8Oco==YRPSa4mq_eZyUcB_Apj-k_vo=7WvTwCp8k+A@mail.gmail.com>
In-Reply-To: <CAKv+Gu8u8Oco==YRPSa4mq_eZyUcB_Apj-k_vo=7WvTwCp8k+A@mail.gmail.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Mon, 7 Oct 2019 09:05:05 -0700
Message-ID: <CALCETrXes_vNPXurB91VH0nZYiZRYrDMDhOy6jrw=tMFiT_nGA@mail.gmail.com>
Subject: Re: [PATCH v2 00/20] crypto: crypto API library interfaces for WireGuard
To:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        David Miller <davem@davemloft.net>,
        Greg KH <gregkh@linuxfoundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Samuel Neves <sneves@dei.uc.pt>,
        Dan Carpenter <dan.carpenter@oracle.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Eric Biggers <ebiggers@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Martin Willi <martin@strongswan.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Mon, Oct 7, 2019 at 8:12 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Mon, 7 Oct 2019 at 17:02, Andy Lutomirski <luto@amacapital.net> wrote:
> >
> > On Sun, Oct 6, 2019 at 10:24 PM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Mon, 7 Oct 2019 at 06:44, Andy Lutomirski <luto@amacapital.net> wrote:
> > > >
> >
> > > > > Actually, this can be addressed by retaining the module dependencies
> > > > > as before, but permitting the arch module to be omitted at load time.
> > > >
> > > > I think that, to avoid surprises, you should refuse to load the arch module if the generic module is loaded, too.
> > > >
> > >
> > > Most arch code depends on CPU features that may not be available given
> > > the context, either because they are SIMD or because they are optional
> > > CPU instructions. So we need both modules at the same time anyway, so
> > > that we can fall back to the generic code at runtime.
> > >
> > > So what I'd like is to have the generic module provide the library
> > > interface, but rely on arch modules that are optional.
> > >
> > > We already have 95% of what we need with weak references. We have the
> > > ability to test for presence of the arch code at runtime, and we even
> > > have code patching for all architectures (through static relocations).
> > >
> > > However, one could argue that this is more a [space] optimization than
> > > anything else, so I am willing to park this discussion until support
> > > for static calls has been merged, and proceed with something simpler.
> >
> > I'd suggest tabling it until static calls are merged.  PeterZ just
> > sent a new patchbomb for it anyway.
> >
>
> As it turns out, static calls are a poor fit for this. Imagine an interface like
>
> poly1305_init(state)
> poly1305_update(state, input)
> poly1305_fina(state, digest)
>
> which can be implemented by different libraries. The problem is that
> state is opaque, and so it is generally not guaranteed that a sequence
> that was started using one implementation can be completed using
> another one.
>
> Since the whole point is having a simple library interface,
> complicating this with RCU hooks or other crazy plumbing to ensure
> that no calls are in progress when you switch one out for another one,
> I don't think static calls are suitable for this use case.
>
> > What I'm trying to get at here and apparently saying badly is that I
> > want to avoid a situation where lsmod shows the arch module loaded but
> > the arch code isn't actually executing.
>
> My goal here is to allow the generic library to be loaded with or
> without the arch code, with the arch code always being used when it is
> loaded. This is what I implemented using weak references, but it
> requires a tweak in the module loader (two lines but not pretty).
> Using weak references preserves the dependency order, since the
> generic module will depend on the arch module (and up the refcount) it
> any of its weak references were fulfilled by the arch module in
> question. Using static calls will invert the dependency relation,
> since the arch code will need to perform a static_call_update() to
> make [users of] the generic library point to its code. How this works
> with managing the module refcounts and unload order is an open
> question afaict.

Indeed.  Dealing with unloading when static calls are in use may be messy.

>
> > Regardless of how everything
> > gets wired up (static calls, weak refs, etc), the system's behavior
> > should match the system's configuration, which means that we should
> > not allow any improper order of loading things so that everything
> > *appears* to be loaded but does not actually function.
> >
>
> Yes. that is the whole point.
>
> > Saying "modprobe will do the right thing and let's not worry about
> > silly admins using insmod directly" is not a good solution.
>
> Agreed.
>
> I have disregarded static calls and weak references for the time
> being, and I will proceed with an implementation that uses neither.
> The downside of this is that, e.g., we are forced to load the NEON
> module on non-NEON capable hardware without calling any of its code,
> but this is basically the Zinc situation only with the NEON and the
> generic code living in different modules.

Makes sense.  It can always be improved after the initial
implementation is merged.