public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Weixie Cui <cuiweixie@gmail.com>, Jiri Olsa <jolsa@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com,
	memxor@gmail.com, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 7.0-6.18] bpf: propagate kvmemdup_bpfptr errors from bpf_prog_verify_signature
Date: Mon, 20 Apr 2026 09:21:07 -0400	[thread overview]
Message-ID: <20260420132314.1023554-273-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Weixie Cui <cuiweixie@gmail.com>

[ Upstream commit ad2f7ed0ee91d63792cbe52f2b38325918ae3daa ]

kvmemdup_bpfptr() returns -EFAULT when the user pointer cannot be
copied, and -ENOMEM on allocation failure. The error path always
returned -ENOMEM, misreporting bad addresses as out-of-memory.

Return PTR_ERR(sig) so user space gets the correct errno.

Signed-off-by: Weixie Cui <cuiweixie@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: https://lore.kernel.org/r/tencent_C9C5B2B28413D6303D505CD02BFEA4708C07@qq.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed. Let me compile the full analysis.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

**Step 1.1: Subject Line**
- Subsystem: `bpf`
- Action verb: "propagate" (indicating a correctness fix for error
  handling)
- Summary: Propagate correct errno from `kvmemdup_bpfptr()` in
  `bpf_prog_verify_signature()`

**Step 1.2: Tags**
- `Signed-off-by: Weixie Cui <cuiweixie@gmail.com>` - author
- `Acked-by: Jiri Olsa <jolsa@kernel.org>` - well-known BPF developer,
  regular contributor
- `Link:` to lore.kernel.org
- `Signed-off-by: Alexei Starovoitov <ast@kernel.org>` - BPF co-
  maintainer applied the patch
- No `Fixes:` tag, no `Cc: stable`, no `Reported-by:` (all expected for
  autosel review)

**Step 1.3: Body Text**
The commit message clearly describes: `kvmemdup_bpfptr()` can return
either `-EFAULT` (bad user pointer) or `-ENOMEM` (allocation failure),
but the error path always returned `-ENOMEM`, misreporting bad addresses
as out-of-memory. The fix returns `PTR_ERR(sig)` to propagate the
correct errno.

**Step 1.4: Hidden Bug Fix?**
This is an explicit correctness fix for error reporting. Not hidden.

Record: [bpf] [propagate/correct] [fixes incorrect errno returned to
userspace from kvmemdup_bpfptr failure]

## PHASE 2: DIFF ANALYSIS

**Step 2.1: Inventory**
- Single file: `kernel/bpf/syscall.c`
- 1 line changed: `-ENOMEM` → `PTR_ERR(sig)`
- Function modified: `bpf_prog_verify_signature()`
- Classification: single-file, single-line surgical fix

**Step 2.2: Code Flow Change**
Before: When `kvmemdup_bpfptr()` returns an error (either `-EFAULT` or
`-ENOMEM`), the function unconditionally returns `-ENOMEM`.
After: The function returns the actual error code from the failed call.

**Step 2.3: Bug Mechanism**
Category: Logic/correctness fix - incorrect error code returned to
userspace. When a user passes a bad pointer for the BPF program
signature, they get `ENOMEM` instead of `EFAULT`.

Verified from `include/linux/bpfptr.h`:

```68:79:include/linux/bpfptr.h
static inline void *kvmemdup_bpfptr_noprof(bpfptr_t src, size_t len)
{
        void *p = kvmalloc_node_align_noprof(len, 1, GFP_USER |
__GFP_NOWARN, NUMA_NO_NODE);

        if (!p)
                return ERR_PTR(-ENOMEM);
        if (copy_from_bpfptr(p, src, len)) {
                kvfree(p);
                return ERR_PTR(-EFAULT);
        }
        return p;
}
```

Other call sites in the same file (`___bpf_copy_key` at line 1700,
`map_update_elem` at line 1806-1808) already correctly propagate
`PTR_ERR()`. This fix brings `bpf_prog_verify_signature` into
consistency.

**Step 2.4: Fix Quality**
- Obviously correct: trivial `PTR_ERR()` idiom, standard kernel pattern
- Minimal: 1 line
- Zero regression risk: only changes which errno is returned, cannot
  break any functionality
- No API changes, no structure changes

## PHASE 3: GIT HISTORY INVESTIGATION

**Step 3.1: Blame**
The buggy line was introduced by commit `34927156830369` (KP Singh,
2025-09-21) - "bpf: Implement signature verification for BPF programs".
This was a v7 patch series for signed BPF programs. The incorrect
`-ENOMEM` has been present since the function's introduction.

**Step 3.2: Fixes tag**
No Fixes: tag present. The bug was introduced in `34927156830369`
(v6.18).

**Step 3.3: File History**
The file is actively maintained (many recent commits). No related fixes
for this specific issue found.

**Step 3.4: Author**
Weixie Cui is not a frequent BPF contributor (no other commits found in
the tree). However, the patch was acked by Jiri Olsa (major BPF
developer) and applied by Alexei Starovoitov (BPF co-maintainer).

**Step 3.5: Dependencies**
None. This is a completely standalone one-line fix.

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

**Step 4.1-4.2:** Lore.kernel.org is behind Anubis PoW protection, so
WebFetch fails. b4 dig found the original series that introduced the
function (v1 through v7 of the "Signed BPF programs" series). The fix
commit itself is not yet in the tree being analyzed (it's a candidate
for backport).

**Step 4.3:** No Reported-by tag. This is a code-review-found bug
(author noticed incorrect error propagation).

**Step 4.4-4.5:** Could not access lore due to bot protection. No series
dependencies for the fix.

## PHASE 5: CODE SEMANTIC ANALYSIS

**Step 5.1-5.4: Call Chain**
`__sys_bpf()` (syscall handler) → `bpf_prog_load()` →
`bpf_prog_verify_signature()` → `kvmemdup_bpfptr()`

This is directly reachable from the BPF syscall (`BPF_PROG_LOAD`
command) when `attr->signature` is set. Any userspace program loading a
signed BPF program can trigger this code path.

**Step 5.5: Similar Patterns**
The other two `kvmemdup_bpfptr()` callsites in the same file correctly
use `PTR_ERR()`. This is the only inconsistent callsite.

## PHASE 6: STABLE TREE ANALYSIS

**Step 6.1:** The `bpf_prog_verify_signature` function was introduced in
commit `34927156830369` which first appeared in v6.18. Verified:
- NOT in v6.12, v6.13, v6.14, v6.15 (exit code 1 from `merge-base --is-
  ancestor`)
- IS in v6.18, v6.19, v7.0

So only 6.18.y, 6.19.y, and 7.0.y stable trees are affected (if active).

**Step 6.2:** The fix would apply cleanly to 7.0.y. For 6.18.y/6.19.y,
the context differs slightly (missing the `KMALLOC_MAX_CACHE_SIZE` check
from `ea1535e28bb377` which is only in v7.0), but the changed line
itself is identical.

**Step 6.3:** No related fixes already in stable.

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

**Step 7.1:** BPF subsystem (`kernel/bpf/`) - IMPORTANT level. BPF is
widely used for networking, security, tracing, etc. The signature
verification feature specifically serves security use cases.

**Step 7.2:** Actively maintained subsystem with frequent commits.

## PHASE 8: IMPACT AND RISK ASSESSMENT

**Step 8.1:** Affected users: those loading signed BPF programs
(security-conscious environments using BPF program signature
verification).

**Step 8.2:** Trigger: pass an invalid signature pointer via
`BPF_PROG_LOAD`. Unprivileged users can trigger this (if they have BPF
access). Not a crash path - just wrong errno.

**Step 8.3:** Failure mode: Incorrect errno returned to userspace
(`ENOMEM` instead of `EFAULT`). Severity: **LOW**. No crash, no data
corruption, no security vulnerability. But misleading error codes can
cause tools to take incorrect recovery actions (e.g., backing off for
memory pressure instead of reporting a programming error).

**Step 8.4:**
- Benefit: LOW-MEDIUM (correct errno for BPF signature verification
  users)
- Risk: NEAR-ZERO (1 line, standard `PTR_ERR()` pattern, cannot regress)
- Ratio: Favorable - benefit > risk

## PHASE 9: FINAL SYNTHESIS

**Evidence FOR backporting:**
- Fixes a real bug (incorrect errno to userspace)
- Trivially correct one-line fix using standard kernel idiom
- Zero regression risk
- Acked by major BPF developer (Jiri Olsa), applied by BPF maintainer
  (Alexei Starovoitov)
- Brings code into consistency with other `kvmemdup_bpfptr()` callsites
- Directly reachable from syscall path
- Standalone, no dependencies

**Evidence AGAINST backporting:**
- Low severity: incorrect errno, not a crash/corruption/security issue
- Only affects relatively new code (v6.18+), limited to a few stable
  trees
- No Reported-by, no evidence users have been affected in practice
- Stable rules ask for "important issues" - wrong errno is debatable

**Stable Rules Checklist:**
1. Obviously correct and tested? YES
2. Fixes a real bug? YES (incorrect errno)
3. Important issue? BORDERLINE - incorrect errno is a real bug but low
   severity
4. Small and contained? YES (1 line)
5. No new features or APIs? YES
6. Can apply to stable trees? YES (clean apply to 7.0.y)

## Verification

- [Phase 1] Parsed tags: Acked-by Jiri Olsa, SOB by Alexei Starovoitov
- [Phase 2] Diff analysis: 1 line changed, `-ENOMEM` -> `PTR_ERR(sig)`
  in `bpf_prog_verify_signature()`
- [Phase 2] Verified `kvmemdup_bpfptr` returns `-EFAULT` or `-ENOMEM`
  from `include/linux/bpfptr.h:68-79`
- [Phase 2] Verified other callsites (lines 1700, 1806-1808) already use
  `PTR_ERR()` correctly
- [Phase 3] git blame: buggy code introduced in `34927156830369` (KP
  Singh, 2025-09-21, v6.18)
- [Phase 3] Verified `34927156830369` NOT in v6.12 (exit 1), IS in v6.18
  (exit 0)
- [Phase 4] b4 dig found original series at lore (v7, "Signed BPF
  programs" series)
- [Phase 4] lore.kernel.org inaccessible via WebFetch (Anubis
  protection)
- [Phase 5] Call chain: `__sys_bpf()` -> `bpf_prog_load()` (line 6249)
  -> `bpf_prog_verify_signature()` (line 3034) - verified via grep
- [Phase 6] Bug exists only in v6.18+, NOT in v6.12 or earlier LTS trees
- [Phase 8] Severity: LOW (incorrect errno, no crash/corruption)
- UNVERIFIED: Could not access lore discussion to check if stable was
  discussed by reviewers

This is a borderline case. The fix is a trivially correct one-line
change with zero regression risk, fixing a genuine incorrect errno bug
in the BPF syscall path. While the severity is low (wrong error code,
not a crash), the near-zero risk makes the risk-benefit ratio favorable.
The incorrect errno could mislead BPF tools into treating a bad pointer
as a memory pressure issue.

**YES**

 kernel/bpf/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 700938782bed2..cbb8696c04b59 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2832,7 +2832,7 @@ static int bpf_prog_verify_signature(struct bpf_prog *prog, union bpf_attr *attr
 	sig = kvmemdup_bpfptr(usig, attr->signature_size);
 	if (IS_ERR(sig)) {
 		bpf_key_put(key);
-		return -ENOMEM;
+		return PTR_ERR(sig);
 	}
 
 	bpf_dynptr_init(&sig_ptr, sig, BPF_DYNPTR_TYPE_LOCAL, 0,
-- 
2.53.0


  parent reply	other threads:[~2026-04-20 13:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:16 ` [PATCH AUTOSEL 6.18] xsk: fix XDP_UMEM_SG_FLAG issues Sasha Levin
2026-04-20 13:18 ` [PATCH AUTOSEL 6.18] xsk: tighten UMEM headroom validation to account for tailroom and min frame Sasha Levin
2026-04-21 17:29   ` sashiko-bot
2026-04-20 13:19 ` [PATCH AUTOSEL 7.0-5.10] bpf: Do not increment tailcall count when prog is NULL Sasha Levin
2026-04-20 21:22   ` sashiko-bot
2026-04-20 13:20 ` [PATCH AUTOSEL 6.18] xsk: respect tailroom for ZC setups Sasha Levin
2026-04-20 13:20 ` [PATCH AUTOSEL 7.0-6.6] s390/bpf: Do not increment tailcall count when prog is NULL Sasha Levin
2026-04-20 13:21 ` Sasha Levin [this message]
2026-04-20 13:21 ` [PATCH AUTOSEL 6.18] xsk: validate MTU against usable frame size on bind Sasha Levin
2026-04-21 18:02   ` sashiko-bot

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=20260420132314.1023554-273-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cuiweixie@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=memxor@gmail.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox