MIME-Version: 1.0
References: <20211223141113.1240679-1-Jason@zx2c4.com> <CAK7LNARjb4=9JOVDdfUg1sxLG4OH-Eko44iEewbr+0Wd+CNf1Q@mail.gmail.com>
In-Reply-To: <CAK7LNARjb4=9JOVDdfUg1sxLG4OH-Eko44iEewbr+0Wd+CNf1Q@mail.gmail.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Sat, 25 Dec 2021 11:26:32 +0100
Message-ID: <CAMj1kXG7PFpj1Oz1osVCBW1NhCHQT+Oc=0WMLYSeRtC1QK4e8Q@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] lib/crypto: blake2s: include as built-in
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     "Jason A. Donenfeld" <Jason@zx2c4.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Theodore Ts'o" <tytso@mit.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, 25 Dec 2021 at 10:28, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Dec 23, 2021 at 11:11 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > In preparation for using blake2s in the RNG, we change the way that it
> > is wired-in to the build system. Instead of kconfig mazes and ifdefs, we
> > use weak symbols, so that an arch version can override the generic
> > version. Then we include the generic version in lib-y, so that it can be
> > removed from the image if the arch version doesn't fallback to it (as is
> > the case on arm though not x86).
>
>
> As I replied in another email, this does not work like that.
>
> Since 7273ad2b08f8ac9563579d16a3cf528857b26f49,
> libs-y are all linked when CONFIG_MODULES=y.
>
>
>
> So, what this patch is doing are:
>
>  - Add __weak to the generic function
>  - Make modules into built-in.
>
>
> Both generic functions and ARM-specific ones
> will remain in vmlinux.
>
> __weak makes it difficult to track which function is
> actually used.
> Using #ifdef CONFIG_* (as the current code does)
> is better.
>
>
>
> >
> > diff --git a/Makefile b/Makefile
> > index d85f1ff79f5c..892ea632ea63 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -668,7 +668,7 @@ drivers-y   := drivers/ sound/
> >  drivers-$(CONFIG_SAMPLES) += samples/
> >  drivers-$(CONFIG_NET) += net/
> >  drivers-y      += virt/
> > -libs-y         := lib/
> > +libs-y         := lib/ lib/crypto/
>
>
> If this is merged, someone will try to
> add random patterns.
> libs-y         := lib/ lib/crypto/ lib/foo/bar/ lib/foo/ba/baz
>
>
>
> lib-y and libs-y are a bad idea in the first place
> and should not be extended any more.
>
> Since this patch is not working as the commit description
> claims, and it is going in the bad direction, so
>
> NACK
>

So we are no longer permitted to use static libraries to provide
routines that should only be pulled into vmlinux on demand? Has this
also changed for things like string routines etc?