Re: [PATCH bpf-next v3 3/9] kallsyms: Add KALLSYMS_QUERY_BY_NAME ioctl to /proc/kallsyms

[bot+bpf-ci@kernel.org]

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

> kallsyms: Add KALLSYMS_QUERY_BY_NAME ioctl to /proc/kallsyms

TL;DR:
- KALLSYMS_QUERY_BY_NAME may scan BPF ksyms quadratically in one syscall with
  no cond_resched(), possibly cancelling the speedup
- module_filter may return -ENOENT for init symbols that the unfiltered query
  finds via the ftrace section

> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c

[ ... ]

> +static int kallsyms_find_name_other(unsigned int sec, unsigned int *idx,
> +				    const struct kallsyms_name_query *q,
> +				    struct kallsyms_sym_match *m)
> +{

[ ... ]

> +	do {
> +		if (sec == KSYM_SEC_BPF)
> +			err = bpf_get_kallsym(*idx, &m->addr, &type, m->name);
> +		else if (sec == KSYM_SEC_KPROBE)
> +			err = kprobe_get_kallsym(*idx, &m->addr, &type, m->name);
> +		else
> +			err = ftrace_mod_get_kallsym(*idx, &m->addr, &type,
> +						     m->name, m->modname,
> +						     &exported);
> +		if (!err && !strncmp(m->name, q->name, q->cmp_len))
> +			return 0;
> +		(*idx)++;
> +	} while (!err);

Is this loop quadratic in the number of BPF ksyms, with no reschedule point?

bpf_get_kallsym() restarts from the list head on every call and walks
forward to the index:

    list_for_each_entry_rcu(ksym, &bpf_kallsyms, lnode) {
        if (it++ != symnum)
            continue;

So one ioctl that scans the BPF section without a match does n(n+1)/2 list
steps inside a single syscall.  Every exhaustive query gets there, since
even a query whose matches are all in vmlinux or modules still scans the
ftrace, BPF and kprobe sections on the call that returns -ENOENT.

The uapi comment tells users to keep calling with pos + 1 until -ENOENT,
and the libbpf user later in this series (822e9da9a9b7 "libbpf: Resolve
typeless ksyms via KALLSYMS_QUERY_BY_NAME ioctl") does this twice per
typeless extern, once for the exact name and once for the "<name>.llvm."
prefix.

With read() of /proc/kallsyms the same getter cost is spread across
syscalls, because seq_read fills at most one seq_file buffer per call.  Here
all of it lands in each call that reaches the BPF section.  On hosts with
thousands of BPF programs and trampolines, does this cancel the speedup the
commit message describes?

On kernels without ARCH_HAS_PREEMPT_LAZY, or with ARCH_NO_PREEMPT, it is
also a long run with no reschedule, whereas kallsyms_on_each_symbol() and
kallsyms_on_each_match_symbol() in this file call cond_resched().

Would a helper that walks bpf_kallsyms once from a start index be better,
or at least a cond_resched() in this loop?  ftrace_mod_get_kallsym() has the
same restart-from-the-beginning pattern, although its lists are usually
small.

[ ... ]

> +	if (karg.module_filter) {
> +		len = kallsyms_copy_str_from_user(bufs->module_filter,
> +						  sizeof(bufs->module_filter),
> +						  karg.module_filter);
> +		if (len < 0)
> +			return len;
> +		if (!strcmp(bufs->module_filter, "vmlinux")) {
> +			end_sec = KSYM_SEC_VMLINUX + 1;
> +		} else {
> +			first_sec = KSYM_SEC_MODULE;
> +			end_sec = KSYM_SEC_MODULE + 1;
> +			q.modname = bufs->module_filter;
> +		}

Can module_filter return -ENOENT for a symbol that the unfiltered query
finds in the same module?

When a module name is given, only KSYM_SEC_MODULE is searched.  But
/proc/kallsyms also lists entries for a loaded module from the ftrace
section.  ftrace_free_mem(), called from do_init_module() for the module's
init text, runs allocate_ftrace_mod_map() when any ftrace_ops is registered,
and ftrace_mod_get_kallsym() returns those entries with
strscpy(module_name, mod_map->mod->name, ...).  They appear as, for example:

    ffffffffc0a01000 T foo_init [foo]

and remain until the module is unloaded, because ftrace_release_mod() frees
the map then.  After init, mod->kallsyms has switched to core_kallsyms, so
these init symbols are no longer in the module section.  That gives:

    BY_NAME("foo_init", module_filter = NULL)  -> match, sym_module = "foo"
    BY_NAME("foo_init", module_filter = "foo") -> -ENOENT

The uapi comment says the ioctl finds a symbol "in the entries of
/proc/kallsyms", so the filtered result does not look like the unfiltered
result limited to [foo].  kallsyms_find_name_other() does not check
q->modname for the ftrace section, so extending end_sec alone would not be
enough.

Should the filtered case also search KSYM_SEC_FTRACE and skip entries whose
m->modname differs from q->modname, or should the uapi comment document that
module_filter only covers the module's own symbol table?


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