All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Fuyu Zhao" <zhaofuyu@vivo.com>
Cc: bpf@vger.kernel.org
Subject: Re: [RFC PATCH bpf-next 1/2] libbpf: support selective kernel module BTF loading via .kmod_btfs section
Date: Thu, 06 Aug 2026 04:33:05 +0000	[thread overview]
Message-ID: <20260806043305.D488B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260806042042.3239428-2-zhaofuyu@vivo.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Transient stack pointer overwrites persistent hashmap key in is_kmod_btf_needed(), breaking hashmap integrity on bucket collisions.
- [Medium] Memory allocation failure for `obj->kmod_btfs->data` silently returns success (0), masking the error and corrupting intended behavior.
- [Medium] Processing multiple `.kmod_btfs` sections overwrites previous allocations without freeing them.
--

--- Patch [1]: [RFC PATCH bpf-next 1/2] libbpf: support selective kernel module BTF loading via .kmod_btfs section ---
commit 5306ac18160e3a8c2a0f3dd2eda202169f998df6
Author: Fuyu Zhao <zhaofuyu@vivo.com>

libbpf: support selective kernel module BTF loading via .kmod_btfs section

Add support for a new ELF section ".kmod_btfs" that allows BPF programs
to declare which kernel modules need BTF loading. This avoids loading
all module BTFs and speeds up program load when only a subset of modules
is needed.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514e4e9daa825..5cefe82e4a679 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -901,6 +916,68 @@ bpf_object__init_prog(struct bpf_object *obj, struct bpf_program *prog,
>  	return -ENOMEM;
>  }
>  
> +static size_t mod_name_hash_fn(long key, void *ctx)
> +{
> +	return str_hash((char *)key);
> +}
> +
> +static bool mod_name_equal_fn(long key1, long key2, void *ctx)
> +{
> +	return strcmp((char *)key1, (char *)key2) == 0;
> +}
> +
> +static int
> +bpf_object__collect_kmod_btf_names(struct bpf_object *obj, Elf_Data *sec_data,
> +				   const char *sec_name)
> +{
> +	int module_cnt, i, err = 0;
> +
> +	if (sec_data->d_size % KMOD_NAME_LEN != 0) {
> +		pr_warn("sec '%s': size %zu should be multiple of %d\n",
> +			sec_name, sec_data->d_size, KMOD_NAME_LEN);
> +		return -EINVAL;
> +	}
> +
> +	module_cnt = sec_data->d_size / KMOD_NAME_LEN;
> +	obj->kmod_btfs = calloc(1, sizeof(*obj->kmod_btfs));

[Severity: Medium]
If a crafted ELF file contains multiple .kmod_btfs sections, could this calloc
overwrite a previously allocated obj->kmod_btfs and cause a memory leak of
the hashmap and data arrays?

> +	if (!obj->kmod_btfs)
> +		return -ENOMEM;
> +
> +	obj->kmod_btfs->data = calloc(module_cnt, KMOD_NAME_LEN);
> +	if (!obj->kmod_btfs->data)
> +		goto err_out;

[Severity: Medium]
Does this error path silently return success?

Because err is initialized to 0, this goto err_out will return 0 instead of
-ENOMEM. This causes obj->kmod_btfs to be freed in the cleanup path while
the function signals success, making load_module_btfs() fall back to loading
all module BTFs.

> +	memcpy(obj->kmod_btfs->data, sec_data->d_buf, sec_data->d_size);

[ ... ]

> @@ -5803,6 +5884,21 @@ int bpf_core_add_cands(struct bpf_core_cand *local_cand,
>  	return 0;
>  }
>  
> +static bool is_kmod_btf_needed(struct bpf_object *obj, const char *name)
> +{
> +	uintptr_t val;
> +
> +	if (!hashmap__find(obj->kmod_btfs->hashmap, name, &val))
> +		return false;
> +
> +	if (val == KMODS_BTF_LOADED)
> +		return false;
> +
> +	hashmap__set(obj->kmod_btfs->hashmap, name, KMODS_BTF_LOADED, NULL, NULL);

[Severity: Medium]
Can this hashmap__set() corrupt the hashmap by storing a transient stack
pointer as the key?

Looking at load_module_btfs(), name is a local stack array passed to
is_kmod_btf_needed() on every iteration:

load_module_btfs() {
    char name[64];
    ...
        if (obj->kmod_btfs && obj->kmod_btfs->hashmap &&
            !is_kmod_btf_needed(obj, name)) {
    ...
}

Since hashmap__set() only stores the pointer address, all entries will
share the same pointer to the local name array. If a bucket collision
occurs, strcmp(key1, key2) will evaluate to 0 since both point to the
same stack address, falsely matching existing entries and skipping the
requested module.

> +	obj->kmod_btfs->nr_loaded++;
> +	return true;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806042042.3239428-1-zhaofuyu@vivo.com?part=1

  reply	other threads:[~2026-08-06  4:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06  4:20 [RFC PATCH bpf-next 0/2] libbpf: Improve BPF load performance by selectively loading kmod BTFs Fuyu Zhao
2026-08-06  4:20 ` [RFC PATCH bpf-next 1/2] libbpf: support selective kernel module BTF loading via .kmod_btfs section Fuyu Zhao
2026-08-06  4:33   ` sashiko-bot [this message]
2026-08-06  8:38     ` Fuyu Zhao
2026-08-06  4:20 ` [RFC PATCH bpf-next 2/2] selftests/bpf: add tests for selective kmod BTF loading Fuyu Zhao
2026-08-06  4:29   ` sashiko-bot
2026-08-06  8:39     ` Fuyu Zhao
2026-08-07 22:18 ` [RFC PATCH bpf-next 0/2] libbpf: Improve BPF load performance by selectively loading kmod BTFs Andrii Nakryiko
2026-08-10  6:20   ` Fuyu Zhao
2026-08-10 12:27     ` Alan Maguire
2026-08-11  3:41       ` Fuyu Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806043305.D488B1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=zhaofuyu@vivo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.