Re: [PATCH bpf-next v2 1/6] kallsyms: Add KALLSYMS_QUERY ioctl to /proc/kallsyms

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3742 bytes --]

> kallsyms: Add KALLSYMS_QUERY ioctl to /proc/kallsyms

TL;DR: KALLSYMS_QUERY_BY_ADDR may return BPF JIT symbol names that
bpf_jit_kallsyms=0 or bpf_jit_harden hide in /proc/kallsyms, possibly leaking
JIT image locations to openers with show_value.

> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index aec2f06858af..7caaf3e421c7 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c

[ ... ]

> @@ -922,11 +923,142 @@ const char *kdb_walk_kallsyms(loff_t *pos)

[ ... ]

> +static int do_kallsyms_query(struct kallsym_iter *iter, void __user *uarg)
> +{

[ ... ]

> +
> +	/*
> +	 * The text interface shows zero addresses to openers without permission.
> +	 * A zeroed point query result is useless (and an address-to-symbol query
> +	 * would leak KASLR), so fail instead.
> +	 */
> +	if (!iter->show_value)

[ ... ]

> +	case KALLSYMS_QUERY_BY_ADDR:
> +		if (!karg.sym_name_size)
> +			return -EINVAL;
> +		addr = karg.sym_addr;
> +		/* Reject addresses that do not fit in unsigned long (32-bit). */
> +		if (addr != karg.sym_addr)
> +			return -ENOENT;
> +		if (!kallsyms_lookup(addr, NULL, NULL, NULL, namebuf))
> +			return -ENOENT;

Can KALLSYMS_QUERY_BY_ADDR hand out BPF JIT symbol names that /proc/kallsyms
is configured to hide?

kallsyms_lookup() reaches kallsyms_lookup_buildid() and falls through to
bpf_address_lookup() -> bpf_ksym_find(), which searches bpf_tree without any
policy check.  bpf_ksym_add() always inserts into bpf_tree
(bpf_prog_kallsyms_add() only requires bpf_prog_kallsyms_candidate() and
CAP_BPF).

The text listing of the same file goes through get_ksymbol_bpf() ->
bpf_get_kallsym(), which starts with:

    if (!bpf_jit_kallsyms_enabled())
        return ret;

bpf_jit_kallsyms_enabled() returns false when net.core.bpf_jit_harden is
non-zero or net.core.bpf_jit_kallsyms is 0.  bpf_get_kallsym() is the only
caller of bpf_jit_kallsyms_enabled() in the tree, so hiding the names in
/proc/kallsyms is now the only thing these sysctls do.  Commit af91acbc6299
("bpf: Fix bpf jit kallsym access") says so directly:

    It should only skip addr->name resolution for 'cat /proc/kallsyms'.
    That's what bpf_jit_kallsyms and bpf_jit_harden protect.

With this patch, a process whose open of /proc/kallsyms has show_value set
can call ioctl(KALLSYMS_QUERY) with query_flags=KALLSYMS_QUERY_BY_ADDR and
get back names like bpf_prog_<tag>_<name> (plus trampolines, dispatchers and
__cfi_ prefixes) for addresses inside JIT images.  That gives it an
address->name oracle for finding JIT image locations, which these sysctls
are meant to withhold from /proc/kallsyms.

The comment above the show_value check argues for matching the text
interface ("The text interface shows zero addresses to openers without
permission ... an address-to-symbol query would leak KASLR, so fail
instead"), but the code applies only the show_value gate and skips the
BPF-specific gate.  The uapi comment and the commit message both say
BY_ADDR covers "BPF JIT images", but neither mentions bpf_jit_kallsyms or
bpf_jit_harden.

This is reachable whenever CONFIG_KALLSYMS and CONFIG_BPF_JIT are enabled, a
JITed program loaded with CAP_BPF exists, and either sysctl hides BPF
symbols.  No later commit in the series changes kernel/ code to address
this.

Should the BY_ADDR path return -ENOENT when kallsyms_lookup() resolves to a
BPF address (for example, is_bpf_text_address(addr) is true and the address
is not in vmlinux or a module) unless bpf_jit_kallsyms_enabled() is true?


---
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/39878338337