Re: [PATCH bpf-next v2 03/14] bpf: Allow struct_ops prog to return referenced kptr

[Ming Lei <ming.lei@redhat.com>]

Hello Amery,

On Fri, Dec 20, 2024 at 11:55:29AM -0800, Amery Hung wrote:
> From: Amery Hung <amery.hung@bytedance.com>
> 
> Allow a struct_ops program to return a referenced kptr if the struct_ops
> operator's return type is a struct pointer. To make sure the returned
> pointer continues to be valid in the kernel, several constraints are
> required:
> 
> 1) The type of the pointer must matches the return type
> 2) The pointer originally comes from the kernel (not locally allocated)
> 3) The pointer is in its unmodified form
> 
> Implementation wise, a referenced kptr first needs to be allowed to _leak_
> in check_reference_leak() if it is in the return register. Then, in
> check_return_code(), constraints 1-3 are checked. During struct_ops
> registration, a check is also added to warn about operators with
> non-struct pointer return.
> 
> In addition, since the first user, Qdisc_ops::dequeue, allows a NULL
> pointer to be returned when there is no skb to be dequeued, we will allow
> a scalar value with value equals to NULL to be returned.
> 
> In the future when there is a struct_ops user that always expects a valid
> pointer to be returned from an operator, we may extend tagging to the
> return value. We can tell the verifier to only allow NULL pointer return
> if the return value is tagged with MAY_BE_NULL.
> 
> Signed-off-by: Amery Hung <amery.hung@bytedance.com>

I feel this patchset is very useful, as Alexei shared, it can help to
mark two ublk bpf aio kfunc[1] as KF_ACQ/REL for avoiding bpf aio leak.

[1] https://lore.kernel.org/linux-block/20250107120417.1237392-1-tom.leiming@gmail.com/

So I try to test it with ublk bpf patchset, however, looks it fails ublk
bpf selftests. And the test does succeed without applying this patch,
and ublk is built as module.

- apply the 1st and the 3rd(this one) patch
- apply ublk bpf patchset[1]
- build kernel & reboot
- make -C tools/testing/selftests TARGETS=ublk run_tests

make: Entering directory '/root/git/linux/tools/testing/selftests'
make[1]: Entering directory '/root/git/linux/tools/testing/selftests/ublk'
  GEN      vmlinux.h
  CLNG-BPF ublk_loop.bpf.o
  GEN-SKEL ublk_loop.skel.h
  CLNG-BPF ublk_null.bpf.o
  GEN-SKEL ublk_null.skel.h
  CLNG-BPF ublk_stripe.bpf.o
  GEN-SKEL ublk_stripe.skel.h
  CC       ublk_bpf.o
  BINARY   ublk_bpf
rm /root/git/linux/tools/testing/selftests/ublk/ublk_bpf.o
make[1]: Leaving directory '/root/git/linux/tools/testing/selftests/ublk'
make[1]: Entering directory '/root/git/linux/tools/testing/selftests/ublk'
TAP version 13
1..8
# timeout set to 45
# selftests: ublk: test_null_01.sh
# null_01 : [PASS]
ok 1 selftests: ublk: test_null_01.sh
# timeout set to 45
# selftests: ublk: test_null_02.sh
# libbpf: struct_ops init_kern: struct ublk_bpf_ops data is not found in struct bpf_struct_ops_ublk_bpf_ops
# libbpf: failed to load object 'ublk_null.bpf.o'
# fail to load bpf obj from ublk_null.bpf.o
# fail to register bpf prog null ublk_null.bpf.o
not ok 2 selftests: ublk: test_null_02.sh # exit=255



Thanks,
Ming