[PATCH bpf-next 0/4] Use correct destructor kfunc types

Sami Tolvanen posted 4 patches 2 months, 1 week ago
There is a newer version of this series
kernel/bpf/btf.c                                     | 7 +++++++
kernel/bpf/crypto.c                                  | 7 ++++++-
net/sched/bpf_qdisc.c                                | 7 ++++++-
tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 7 ++++++-
4 files changed, 25 insertions(+), 3 deletions(-)
[PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Sami Tolvanen 2 months, 1 week ago
Hi folks,

While running BPF self-tests with CONFIG_CFI_CLANG (Clang Control
Flow Integrity) enabled, I ran into a couple of CFI failures
in bpf_obj_free_fields() caused by type mismatches between
the btf_dtor_kfunc_t function pointer type and the registered
destructor functions.

It looks like we can't change the argument type for these
functions to match btf_dtor_kfunc_t because the verifier doesn't
like void pointer arguments for functions used in BPF programs,
so this series fixes the issue by adding stubs with correct types
to use as destructors for each instance of this I found in the
kernel tree.

The last patch changes btf_check_dtor_kfuncs() to enforce the
function type when CFI is enabled, so we don't end up registering
destructors that panic the kernel. Perhaps this is something we
could enforce even without CONFIG_CFI_CLANG?

Sami

---

Sami Tolvanen (4):
  bpf: crypto: Use the correct destructor kfunc type
  bpf: net_sched: Use the correct destructor kfunc type
  selftests/bpf: Use the correct destructor kfunc type
  bpf, btf: Enforce destructor kfunc type with CFI

 kernel/bpf/btf.c                                     | 7 +++++++
 kernel/bpf/crypto.c                                  | 7 ++++++-
 net/sched/bpf_qdisc.c                                | 7 ++++++-
 tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 7 ++++++-
 4 files changed, 25 insertions(+), 3 deletions(-)


base-commit: 95993dc3039e29dabb9a50d074145d4cb757b08b
-- 
2.50.1.470.g6ba607880d-goog
Re: [PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Yonghong Song 2 months, 1 week ago

On 7/24/25 3:32 PM, Sami Tolvanen wrote:
> Hi folks,
>
> While running BPF self-tests with CONFIG_CFI_CLANG (Clang Control
> Flow Integrity) enabled, I ran into a couple of CFI failures
> in bpf_obj_free_fields() caused by type mismatches between
> the btf_dtor_kfunc_t function pointer type and the registered
> destructor functions.
>
> It looks like we can't change the argument type for these
> functions to match btf_dtor_kfunc_t because the verifier doesn't
> like void pointer arguments for functions used in BPF programs,
> so this series fixes the issue by adding stubs with correct types
> to use as destructors for each instance of this I found in the
> kernel tree.
>
> The last patch changes btf_check_dtor_kfuncs() to enforce the
> function type when CFI is enabled, so we don't end up registering
> destructors that panic the kernel. Perhaps this is something we
> could enforce even without CONFIG_CFI_CLANG?

I tried your patch set on top of latest bpf-next. The problem
still exists with the following error:

[   71.976265] CFI failure at bpf_obj_free_fields+0x298/0x620 (target: __bpf_crypto_ctx_release+0x0/0x10; expected type: 0xc1113566)
[   71.980134] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
...


The following is the CFI related config items:

$ grep CFI .config
CONFIG_CFI_AUTO_DEFAULT=y
CONFIG_FUNCTION_PADDING_CFI=11
CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
CONFIG_ARCH_USES_CFI_TRAPS=y
CONFIG_CFI_CLANG=y
# CONFIG_CFI_ICALL_NORMALIZE_INTEGERS is not set
CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG=y
CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC=y
# CONFIG_CFI_PERMISSIVE is not set

Did I miss anything?

>
> Sami
>
> ---
>
> Sami Tolvanen (4):
>    bpf: crypto: Use the correct destructor kfunc type
>    bpf: net_sched: Use the correct destructor kfunc type
>    selftests/bpf: Use the correct destructor kfunc type
>    bpf, btf: Enforce destructor kfunc type with CFI
>
>   kernel/bpf/btf.c                                     | 7 +++++++
>   kernel/bpf/crypto.c                                  | 7 ++++++-
>   net/sched/bpf_qdisc.c                                | 7 ++++++-
>   tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 7 ++++++-
>   4 files changed, 25 insertions(+), 3 deletions(-)
>
>
> base-commit: 95993dc3039e29dabb9a50d074145d4cb757b08b
Re: [PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Sami Tolvanen 2 months, 1 week ago
Hi,

On Fri, Jul 25, 2025 at 9:05 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> I tried your patch set on top of latest bpf-next. The problem
> still exists with the following error:
>
> [   71.976265] CFI failure at bpf_obj_free_fields+0x298/0x620 (target: __bpf_crypto_ctx_release+0x0/0x10; expected type: 0xc1113566)
> [   71.980134] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> ...
>
>
> The following is the CFI related config items:
>
> $ grep CFI .config
> CONFIG_CFI_AUTO_DEFAULT=y
> CONFIG_FUNCTION_PADDING_CFI=11
> CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
> CONFIG_ARCH_USES_CFI_TRAPS=y
> CONFIG_CFI_CLANG=y
> # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS is not set
> CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG=y
> CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC=y
> # CONFIG_CFI_PERMISSIVE is not set
>
> Did I miss anything?

Interesting. I tested this on arm64 and confirmed that the issue is
fixed there, so I wonder if we need to use KCFI_REFERENCE() here to
make sure objtool / x86 runtime patching knows this function actually
indirectly called. I'll test this on x86 and see what's going on.

Thanks for taking a look!

Sami
Re: [PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Yonghong Song 2 months, 1 week ago

On 7/25/25 9:22 AM, Sami Tolvanen wrote:
> Hi,
>
> On Fri, Jul 25, 2025 at 9:05 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>> I tried your patch set on top of latest bpf-next. The problem
>> still exists with the following error:
>>
>> [   71.976265] CFI failure at bpf_obj_free_fields+0x298/0x620 (target: __bpf_crypto_ctx_release+0x0/0x10; expected type: 0xc1113566)
>> [   71.980134] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>> ...
>>
>>
>> The following is the CFI related config items:
>>
>> $ grep CFI .config
>> CONFIG_CFI_AUTO_DEFAULT=y
>> CONFIG_FUNCTION_PADDING_CFI=11
>> CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
>> CONFIG_ARCH_USES_CFI_TRAPS=y
>> CONFIG_CFI_CLANG=y
>> # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS is not set
>> CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG=y
>> CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC=y
>> # CONFIG_CFI_PERMISSIVE is not set
>>
>> Did I miss anything?
> Interesting. I tested this on arm64 and confirmed that the issue is
> fixed there, so I wonder if we need to use KCFI_REFERENCE() here to
> make sure objtool / x86 runtime patching knows this function actually
> indirectly called. I'll test this on x86 and see what's going on.

I just tried arm64 with your patch set. CFI crash still happened:

CFI failure at tcp_ack+0xe74/0x13cc (target: bpf__tcp_congestion_ops_in_ack_event+0x0/0x78; expected type: 0x64424
87a)
Internal error: Oops - CFI: 00000000f2008228 [#1]  SMP
Modules linked in: bpf_testmod(OE) [last unloaded: bpf_testmod(OE)]
CPU: 0 UID: 0 PID: 152 Comm: test_progs Tainted: G           OE       6.16.0-rc6-g95993dc3039e-dirty #162 NONE
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: linux,dummy-virt (DT)
pstate: 33400005 (nzCV daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
pc : tcp_ack+0xe74/0x13cc
lr : tcp_ack+0xe34/0x13cc

The arm64 CFI related config:
$ cat .config | grep CFI
CONFIG_AS_HAS_CFI_NEGATE_RA_STATE=y
CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
CONFIG_CFI_CLANG=y
# CONFIG_CFI_ICALL_NORMALIZE_INTEGERS is not set
CONFIG_HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG=y
# CONFIG_CFI_PERMISSIVE is not set

>
> Thanks for taking a look!
>
> Sami

Re: [PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Sami Tolvanen 2 months, 1 week ago
On Fri, Jul 25, 2025 at 9:54 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> I just tried arm64 with your patch set. CFI crash still happened:
>
> CFI failure at tcp_ack+0xe74/0x13cc (target: bpf__tcp_congestion_ops_in_ack_event+0x0/0x78; expected type: 0x64424
> 87a)

This one should fixed by the other series I posted earlier:

https://lore.kernel.org/bpf/20250722205357.3347626-5-samitolvanen@google.com/

Sami
Re: [PATCH bpf-next 0/4] Use correct destructor kfunc types
Posted by Yonghong Song 2 months, 1 week ago

On 7/25/25 10:20 AM, Sami Tolvanen wrote:
> On Fri, Jul 25, 2025 at 9:54 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>> I just tried arm64 with your patch set. CFI crash still happened:
>>
>> CFI failure at tcp_ack+0xe74/0x13cc (target: bpf__tcp_congestion_ops_in_ack_event+0x0/0x78; expected type: 0x64424
>> 87a)
> This one should fixed by the other series I posted earlier:
>
> https://lore.kernel.org/bpf/20250722205357.3347626-5-samitolvanen@google.com/

Okay, I see. We can delay arm64 for now and focus on x86 side as I can
observe some issues with CONFIG_CFI_CLANG.

>
> Sami