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

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

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