> selftests/bpf: Test BTF sysfs inline representations
>
> Load vmlinux, vmlinux.inline and all module and module.inline
> entries in /sys/fs/btf; this will allow us to sanity-check

The changelog references '/sys/fs/btf', but the directory is
/sys/kernel/btf. The code uses BTF_SYSFS_DIR "/sys/kernel/btf", the
existing call at test_btf_sysfs() uses "/sys/kernel/btf/vmlinux", and
the kernel side (89015dbacfe6) says "available in /sys/kernel/btf".
Should the message say /sys/kernel/btf instead?

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c 
> b/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c
> index 3923e64c4c1d..97eccfd7c134 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c

[ ... ]

> +static void test_btf_inline_sysfs_all(void)
> +{

[ ... ]

> +     while ((dentry = readdir(dir)) != NULL) {
> +             struct btf *base_btf = NULL, *module_btf = NULL, *inline_btf = 
> NULL;
> +             char btf_path[PATH_MAX], inline_path[PATH_MAX];
> +             struct stat st;
> +
> +             /* Skip ".", ".." and "foo.inline" */
> +             if (strstr(dentry->d_name, "."))
> +                     continue;
> +
> +             if (strcmp(dentry->d_name, "vmlinux") == 0)
> +                     base_btf = vmlinux_btf;
> +
> +             if (snprintf(btf_path, sizeof(btf_path), "%s/%s",
> +                          BTF_SYSFS_DIR, dentry->d_name) >= sizeof(btf_path) 
> ||
> +                 snprintf(inline_path, sizeof(inline_path), "%s/%s%s",
> +                          BTF_SYSFS_DIR, dentry->d_name, BTF_INLINE_SUFFIX) 
> >=
> +                          sizeof(inline_path)) {
> +                     ASSERT_FAIL("BTF sysfs path is too long\n");
> +                     break;
> +             }
> +
> +             if (!base_btf) {
> +                     module_btf = btf__parse_split(btf_path, vmlinux_btf);
> +                     err = libbpf_get_error(module_btf);
> +                     if (err) {
> +                             /* A module can be unloaded while its sysfs 
> entry is iterated. */
> +                             if (err == -ENOENT)
> +                                     continue;
> +                             ASSERT_OK(err, "parse_module_btf");
> +                             continue;
> +                     }
> +                     base_btf = module_btf;
> +             }
> +             if (stat(inline_path, &st)) {
> +                     err = errno;
> +                     if (err == ENOENT)
> +                             continue;
> +                     ASSERT_OK(err, "stat_inline_btf");
> +             }

Is there a missing continue after the ASSERT_OK? On a stat() error other
than ENOENT (EACCES, EIO, ELOOP), the assertion records the failure but
then execution falls into the btf__parse_split() call below, which fails
on the same path for the same reason. Every other error branch in this
function continues; this is the only fall-through. Compare also
test_btf_mmap_sysfs() where every failed check goes to cleanup.

> +             inline_btf = btf__parse_split(inline_path, base_btf);
> +             err = libbpf_get_error(inline_btf);
> +             if (!err)
> +                     btf__free(inline_btf);
> +             ASSERT_OK(err, "parse_inline_btf");
> +             btf__free(module_btf);
> +     }

The module-unload race documented at the module BTF read is not handled
for the inline BTF read. The comment above notes 'A module can be
unloaded while its sysfs entry is iterated', and the module BTF parse
tolerates err == -ENOENT. The inline parse has no such tolerance:
btf__parse_split() returns NULL with errno = ENOENT if the file has gone
away, and ASSERT_OK(-ENOENT, "parse_inline_btf") fails the test.

The stat() above does not protect this because it is a check performed
strictly before the use (TOCTOU pre-check, not exclusion). Concrete
sequence:

  1. readdir() returns "bpf_test_rqspinlock"
  2. module BTF parse succeeds
  3. stat(inline_path) succeeds
  4. concurrently another test unloads the module:
     prog_tests/res_spin_lock.c unload_module("bpf_test_rqspinlock"),
     or prog_tests/bpf_mod_race.c, or test_progs.c unload_bpf_testmod()
     reaches btf_module_notify(MODULE_STATE_GOING) and calls
     sysfs_btf_remove(btf_mod->sysfs_inline_attr), removing the file
  5. btf__parse_split() open() fails with ENOENT, err = -ENOENT
  6. ASSERT_OK fails, spurious test failure

Could the inline parse apply the same tolerance already used for the
module parse (treat -ENOENT as 'entry vanished, skip')?

Also, with CONFIG_DEBUG_INFO_BTF_INLINE off (the default, and the current
selftests/bpf/config), does this test report PASS without exercising
anything?

CONFIG_DEBUG_INFO_BTF_INLINE is 'default n' (lib/Kconfig.debug) and
'depends on PAHOLE_HAS_INLINE' (pahole >= 1.31). Neither
tools/testing/selftests/bpf/config nor any config.<arch> gains
CONFIG_DEBUG_INFO_BTF_INLINE anywhere in the series (verified over
a04bcbfa9ea1~1..f197212706a6: no commit touches those files).

With the option off, kernel/bpf/btf_vmlinux_inline.c registers nothing
(guarded by IS_BUILTIN(CONFIG_DEBUG_INFO_BTF_INLINE)), so
/sys/kernel/btf/vmlinux.inline does not exist. The module path in
btf_module_notify() is #if IS_ENABLED(CONFIG_DEBUG_INFO_BTF_INLINE), so
no <mod>.inline exists either.

Every readdir() iteration therefore reaches the stat() at the line marked
above, stat() fails with ENOENT, and 'continue' is taken. The loop runs
to completion, no ASSERT_* is ever evaluated against inline BTF, and
test_btf_sysfs is reported as OK. The very feature the series adds is
never touched by the test that is supposed to cover it, in exactly the
configuration the BPF CI builds.

A subsystem pattern flags this as potentially concerning: tests that
assert/fail on EOPNOTSUPP, ENOSYS, or similar "capability absent" errno
values instead of skipping, or that skip silently with no message,
receive this flag. The vacuous pass is not a crash and does not
destabilize the system, but this is the test not running at all in the
default/CI configuration, which is the 'silent pass' failure mode
selftests.md names.

The next patch in the same series (f197212706a6, 'selftests/bpf: Add a
test verifying inline information') explicitly states 'Test is skipped if
pahole does not encode inline info', so the skip idiom is being used
elsewhere in the series.

Could the test track whether any *.inline entry was found and call
test__skip() with a reason if none were, and add
CONFIG_DEBUG_INFO_BTF_INLINE=y to tools/testing/selftests/bpf/config?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33537080133

Reply via email to