All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Daniel Borkmann" <daniel@iogearbox.net>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 2/9] bpf: Move bigger allocations below fd_array resolution
Date: Mon, 06 Jul 2026 14:21:59 +0000	[thread overview]
Message-ID: <20260706142159.A34551F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260706135644.326006-3-daniel@iogearbox.net>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing memory barrier in double-checked locking of `btf_vmlinux`
- [High] Unprivileged users can hold `bpf_verifier_lock` indefinitely via userfaultfd, causing a Denial of Service
--

commit a8c04cef46e77fd3e978148e583b8fbfc045147f
Author: Daniel Borkmann <daniel@iogearbox.net>
Date:   Mon Jul 6 13:56:37 2026 +0000

    bpf: Move bigger allocations below fd_array resolution
    
    Reorder the preparation work in bpf_check() such that only the minimally
    necessary setup happens up front: allocating the env, initializing the
    verifier log and resolving the fd_array that a signed BPF metadata map
    needs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 33f9c47ee197d..e227c76a8dd96 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -19915,21 +19903,33 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  	env->bypass_spec_v4 = bpf_bypass_spec_v4(env->prog->aux->token);
>  	env->bpf_capable = is_priv = bpf_token_capable(env->prog->aux->token, CAP_BPF);
>  
> -	bpf_get_btf_vmlinux();
> -
> -	/* grab the mutex to protect few globals used by verifier */
> -	if (!is_priv)
> -		mutex_lock(&bpf_verifier_lock);
> -
>  	/* user could have requested verbose verifier output
>  	 * and supplied buffer to store the verification trace
>  	 */
>  	ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
>  	if (ret)
> -		goto err_unlock;
> +		goto err_free_env;
>  
>  	ret = process_fd_array(env, attr, uattr);
>  	if (ret)
> +		goto err_prep;
> +
> +	bpf_get_btf_vmlinux();
> +

[Severity: High]
This isn't a bug introduced by this patch, but does the double-checked
locking pattern in bpf_get_btf_vmlinux() lack necessary memory barriers?

kernel/bpf/verifier.c:bpf_get_btf_vmlinux() {
	if (!btf_vmlinux && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
		mutex_lock(&bpf_verifier_lock);
		if (!btf_vmlinux)
			btf_vmlinux = btf_parse_vmlinux();
		mutex_unlock(&bpf_verifier_lock);
	}
	return btf_vmlinux;
}

If btf_vmlinux is read locklessly without smp_load_acquire() or READ_ONCE(),
and the assignment inside the lock lacks smp_store_release(), the CPU could
reorder the initialization of the struct btf object with the store to the
btf_vmlinux pointer.

A concurrent thread calling bpf_get_btf_vmlinux() could observe
btf_vmlinux != NULL and return the pointer before the object's contents are
fully visible, leading to uninitialized memory dereferences.

> +	/* grab the mutex to protect few globals used by verifier */
> +	if (!is_priv)
> +		mutex_lock(&bpf_verifier_lock);

[Severity: High]
This is a pre-existing issue, but can unprivileged users hold
bpf_verifier_lock indefinitely via userfaultfd, causing a local DoS?

When an unprivileged user executes a BPF program load, the verifier acquires
bpf_verifier_lock. While holding this lock, the verifier accesses user-space
memory in a few places.

For example, when reading the user-supplied array in
fd_array_get_map_idx_sparse():

kernel/bpf/verifier.c:fd_array_get_map_idx_sparse() {
	if (copy_from_bpfptr_offset(&fd, env->fd_array_raw, ...))
		return -EFAULT;
	...
}

Similar reads occur in fd_array_get_btf_sparse(). Additionally,
bpf_log_attr_finalize() writes the verification log back to the user-supplied
attr_log->ubuf before the mutex is released on some error paths.

If an unprivileged user maps these user-space buffers with userfaultfd,
they could cause the kernel thread to pause indefinitely in the fault
handler while still holding bpf_verifier_lock.

Since this blocks all other unprivileged BPF program loads system-wide,
could this be exploited to deny service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706135644.326006-1-daniel@iogearbox.net?part=2

  reply	other threads:[~2026-07-06 14:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 13:56 [PATCH bpf-next v4 0/9] Verify BPF signed loader at load time Daniel Borkmann
2026-07-06 13:56 ` [PATCH bpf-next v4 1/9] bpf: Resolve and cache fd_array objects " Daniel Borkmann
2026-07-06 15:42   ` Anton Protopopov
2026-07-06 13:56 ` [PATCH bpf-next v4 2/9] bpf: Move bigger allocations below fd_array resolution Daniel Borkmann
2026-07-06 14:21   ` sashiko-bot [this message]
2026-07-06 13:56 ` [PATCH bpf-next v4 3/9] bpf: Verify signed loader metadata at load time Daniel Borkmann
2026-07-06 14:26   ` sashiko-bot
2026-07-06 14:48     ` Daniel Borkmann
2026-07-06 15:09   ` bot+bpf-ci
2026-07-06 17:16   ` Paul Moore
2026-07-06 13:56 ` [PATCH bpf-next v4 4/9] libbpf: Drop in-loader metadata check for load-time verification Daniel Borkmann
2026-07-06 14:50   ` bot+bpf-ci
2026-07-06 13:56 ` [PATCH bpf-next v4 5/9] bpftool: Check EVP_Digest when computing excl_prog_hash Daniel Borkmann
2026-07-06 13:56 ` [PATCH bpf-next v4 6/9] bpftool: Cover loader metadata with the program signature Daniel Borkmann
2026-07-06 13:56 ` [PATCH bpf-next v4 7/9] selftests/bpf: Adjust bpf_map layout in verifier_map_ptr Daniel Borkmann
2026-07-06 14:27   ` sashiko-bot
2026-07-06 14:30     ` Daniel Borkmann
2026-07-06 13:56 ` [PATCH bpf-next v4 8/9] selftests/bpf: Verify load-time signed loader metadata Daniel Borkmann
2026-07-06 13:56 ` [PATCH bpf-next v4 9/9] Documentation/bpf: Add BPF signing and enforcement doc Daniel Borkmann
2026-07-06 17:13 ` [PATCH bpf-next v4 0/9] Verify BPF signed loader at load time Paul Moore
2026-07-06 17:47   ` Daniel Borkmann
2026-07-06 19:20     ` Paul Moore

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=20260706142159.A34551F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.