Re: [PATCH bpf-next v4] bpf: crypto: Use AES-CBC and AES-ECB libraries

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Sep 23, 2026 at 04:55:03AM +0000, Alexei Starovoitov wrote:
> On Wed Sep 23, 2026 at 4:21 AM UTC, Eric Biggers wrote:
> > On Wed, Sep 23, 2026 at 04:15:13AM +0000, Alexei Starovoitov wrote:
> >> On Wed Sep 23, 2026 at 4:11 AM UTC, Eric Biggers wrote:
> >> > On Wed, Sep 23, 2026 at 04:02:22AM +0000, Alexei Starovoitov wrote:
> >> >> On Wed Sep 23, 2026 at 3:37 AM UTC, sashiko-bot wrote:
> >> >> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >> >> > - [High] bpf: BPF_CRYPTO is silently disabled if AES libraries are modules
> >> >> > --
> >> >> >
> >> >> > commit 1bcee0286f1c7c6c86befd107081ebb8a0fb0eed
> >> >> > Author: Eric Biggers <ebiggers@kernel.org>
> >> >> >
> >> >> > bpf: crypto: Use AES-CBC and AES-ECB libraries
> >> >> >
> >> >> > This commit reimplements BPF crypto using the AES-CBC and AES-ECB library
> >> >> > APIs instead of the lskcipher API. It adds a new BPF_CRYPTO Kconfig option
> >> >> > to conditionally compile crypto.c based on the new AES library dependencies.
> >> >> >
> >> >> >> diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
> >> >> >> index d7d25477ef481..a44ecfa3e9ef5 100644
> >> >> >> --- a/kernel/bpf/Kconfig
> >> >> >> +++ b/kernel/bpf/Kconfig
> >> >> >> @@ -91,6 +91,15 @@ config BPF_UNPRIV_DEFAULT_OFF
> >> >> >>  
> >> >> >>  	  If you are unsure how to answer this question, answer Y.
> >> >> >>  
> >> >> >> +config BPF_CRYPTO
> >> >> >> +	def_bool y
> >> >> >> +	depends on BPF_SYSCALL
> >> >> >> +	depends on CRYPTO_LIB_AES_CBC
> >> >> >> +	depends on CRYPTO_LIB_AES_ECB
> >> >> >
> >> >> > [Severity: High]
> >> >> > Does this dependency logic silently disable BPF_CRYPTO if the AES libraries
> >> >> > are built as modules?
> >> >> >
> >> >> > BPF_CRYPTO is defined as a bool in Kconfig but depends on CRYPTO_LIB_AES_CBC
> >> >> > and CRYPTO_LIB_AES_ECB, which are tristate symbols. Under Kconfig rules, when
> >> >> > a bool depends on a tristate that evaluates to m, the dependency evaluates
> >> >> > to m, which downgrades the bool to n.
> >> >> >
> >> >> > Consequently, kernel/bpf/crypto.c might not be compiled, and the crypto
> >> >> > kfuncs could be silently stripped from the kernel. Existing BPF programs
> >> >> > using crypto kfuncs will fail to load with "unknown kfunc". Because the AES
> >> >> > library symbols lack user prompts, users cannot manually fix this by
> >> >> > explicitly setting them to =y in menuconfig.
> >> >> 
> >> >> The bot is correct. Looks like a regression.
> >> >
> >> > Users can enable the libraries indirectly by setting
> >> > CONFIG_CRYPTO_AES=y, CONFIG_CRYPTO_ECB=y, and CONFIG_CRYPTO_CBC=y in
> >> > their kconfig, as the self-tests config does.
> >> >
> >> > I do not know what you expect.  The libraries themselves do not contain
> >> > independent functionality (besides functions that other things in the
> >> > kernel can call) and thus are hidden symbols themselves, as per the
> >> > usual convention in the kernel.
> >> >
> >> > As I said on v1, if you want a prompt for BPF_CRYPTO, I can add that.  I
> >> > can't find any other example of kfuncs having prompts, though.
> >> 
> >> No. prompt is not necessary.
> >> My question is why disable BPF_CRYPTO when these are modules?
> >
> > If libaes is built as a module, then either bpf_crypto would have to be
> > built as its own module (which we already ruled out on the last thread),
> > or else this code would have to be merged into libaes.  From my
> > perspective putting this functionality in libaes seems weird because
> > this acts more like a user of the crypto code than part of it.  But
> > maybe it would be more aligned with how kfuncs are usually implemented?
> >
> > Note that it's kind of hard to actually build a kernel with libaes as a
> > module anyway, though, due to so many things needing AES support.
> 
> I'm still missing why it's hard to make this kfuncs work with CONFIG_CRYPTO_AES=m

Module code can only be called directly by module code, not built-in
code.  So CONFIG_CRYPTO_LIB_AES=m && CONFIG_BPF_CRYPTO=y cannot work;
you'd get a linker error.  For a module to offer these kfuncs they would
either need to be their own module (e.g. bpf_crypto.ko), or else merged
into the module of the underlying functions (libaes.ko, though again, in
practice libaes is usually built-in anyway for other reasons).

- Eric