Re: [PATCH v4 09/10] selftests/hornet: Add a selftest for the Hornet LSM

[sashiko-bot@kernel.org]

Sashiko AI review found 5 potential issue(s):
- [Medium] Makefile targets lack the `$(OUTPUT)/` prefix, breaking out-of-tree and standard selftest builds.
- [Medium] Hardcoded use of the host's `/sys/kernel/btf/vmlinux` breaks cross-compilation.
- [Medium] The host tool `gen_sig` is compiled using the target compiler, breaking cross-compilation.
- [Medium] Unconditional build dependency on kernel module signing keys.
- [Medium] Incorrect use of `bpf_probe_read_str()` to read user-space memory.
--

commit d3ed81de44873be8726bc1bb9c61643050622647
Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>

selftests/hornet: Add a selftest for the Hornet LSM

This commit adds a new selftest for the Hornet LSM. It contains a testcase
that utilizes light skeleton eBPF loaders and exercises hornet's map
validation.

> diff --git a/tools/testing/selftests/hornet/Makefile b/tools/testing/selftests/hornet/Makefile
> new file mode 100644
> index 0000000000000..432bce59f54e7
> --- /dev/null
> +++ b/tools/testing/selftests/hornet/Makefile

[ ... ]

> +TEST_GEN_PROGS := loader
> +TEST_GEN_FILES := vmlinux.h loader.h trivial.bpf.o map.bin sig.bin insn.bin signed_loader.h
> +$(TEST_GEN_PROGS): LDLIBS += -lbpf
> +$(TEST_GEN_PROGS): $(TEST_GEN_FILES)
> +
> +include ../lib.mk
> +
> +BPF_CFLAGS := -target bpf \
> +	-D__TARGET_ARCH_$(ARCH) \
> +	-I/usr/include/$(shell uname -m)-linux-gnu \
> +	$(KHDR_INCLUDES)
> +
> +vmlinux.h:

Since ../lib.mk prepends $(OUTPUT)/ to all entries in TEST_GEN_FILES and
TEST_GEN_PROGS, will the generic build infrastructure look for targets like
$(OUTPUT)/vmlinux.h and fall back to default implicit rules because the
custom targets lack the $(OUTPUT)/ prefix?

> +	$(BPFTOOL) btf dump file /sys/kernel/btf/vmlinux format c > vmlinux.h

Does this extract the BTF from the host machine's running kernel instead
of the target kernel? On cross-compiled builds, could this generate
mismatched architecture types?

> +
> +trivial.bpf.o: trivial.bpf.c vmlinux.h
> +	$(CLANG) $(CFLAGS) $(BPF_CFLAGS) -c $< -o $@
> +
> +loader.h: trivial.bpf.o
> +	$(BPFTOOL) gen skeleton -S -k $(CERTDIR)/signing_key.pem -i $(CERTDIR)/signing_key.x509 \

Does this create an unconditional build dependency on the kernel module
signing keys?

If the target kernel is configured without module signing, or the selftests
are built on a clean source tree before the keys are generated, will this
cause the build to fail?

> +		-L $< name trivial > $@
> +
> +insn.bin: loader.h
> +	$(SCRIPTSDIR)/extract-insn.sh $< > $@
> +
> +map.bin: loader.h
> +	$(SCRIPTSDIR)/extract-map.sh $< > $@
> +
> +$(OUTPUT)/gen_sig: ../../../../scripts/hornet/gen_sig.c
> +	$(call msg,GEN_SIG,,$@)
> +	$(Q)$(CC) $(shell $(PKG_CONFIG) --cflags libcrypto 2> /dev/null) \
> +		  $< -o $@ \
> +		  $(shell $(PKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto)
> +
> +sig.bin: insn.bin map.bin $(OUTPUT)/gen_sig
> +	$(OUTPUT)/gen_sig --key $(CERTDIR)/signing_key.pem --cert $(CERTDIR)/signing_key.x509 \

Since gen_sig is executed locally on the host to generate sig.bin, will
compiling it with $(CC) and $(PKG_CONFIG) (which point to the target
toolchain) cause an Exec format error during cross-compilation?

Should it be compiled using $(HOSTCC) and host pkg-config instead?

[ ... ]

> diff --git a/tools/testing/selftests/hornet/trivial.bpf.c b/tools/testing/selftests/hornet/trivial.bpf.c
> new file mode 100644
> index 0000000000000..d38c5b53ff932
> --- /dev/null
> +++ b/tools/testing/selftests/hornet/trivial.bpf.c

[ ... ]

> +SEC("tracepoint/syscalls/sys_enter_unlinkat")
> +int handle_enter_unlink(struct trace_event_raw_sys_enter *ctx)
> +{
> +	char filename[128] = { 0 };
> +	struct task_struct *task;
> +	unsigned long start_time = 0;
> +	int pid = bpf_get_current_pid_tgid() >> 32;
> +	char *pathname_ptr = (char *) BPF_CORE_READ(ctx, args[1]);
> +
> +	bpf_probe_read_str(filename, sizeof(filename), pathname_ptr);

Since pathname_ptr is extracted from args[1] of the sys_enter_unlinkat
tracepoint, isn't it a user-space pointer?

On architectures with strict user and kernel address space separation, does
bpf_probe_read_str() fail to read this memory?

Should this use bpf_probe_read_user_str() instead?

> +	task = (struct task_struct *)bpf_get_current_task();
> +	start_time = BPF_CORE_READ(task, start_time);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260416173500.176716-1-bboscaccy@linux.microsoft.com?part=9