[PATCH bpf v1 0/8] Misc bug fixes - part 4

[PATCH bpf v1 0/8] Misc bug fixes - part 4

[Kumar Kartikeya Dwivedi]

A set of miscellaneous fixes for bugs reported by Nicholas, and GPT-5.6
when analyzing those fixes, batched together again. See commit logs for
details. Related rhtab fixes from Yuan Chen and Nuoqi Gui have been
folded into the series.

Kumar Kartikeya Dwivedi (5):
  selftests/bpf: Test timer field on recycled rhtab element
  bpf: Mark NULL kptr stores precise
  selftests/bpf: Test imprecise scalar kptr stores
  bpf: Preserve inner map identity in callback frames
  selftests/bpf: Test inner map identities in callbacks

Nuoqi Gui (2):
  bpf: Cancel special fields when recycling rhtab elements
  selftests/bpf: Test rhtab kptr cancellation semantics

Yuan Chen (1):
  bpf: Preserve special fields in recycled rhtab elements

 kernel/bpf/hashtab.c                          |  18 +--
 kernel/bpf/verifier.c                         |  21 ++-
 .../testing/selftests/bpf/prog_tests/rhash.c  |   6 +
 .../selftests/bpf/prog_tests/rhash_timer.c    | 141 ++++++++++++++++++
 .../selftests/bpf/prog_tests/timer_mim.c      |  29 +++-
 .../selftests/bpf/progs/map_kptr_fail.c       |  37 +++++
 tools/testing/selftests/bpf/progs/rhash.c     | 112 ++++++++++++++
 .../testing/selftests/bpf/progs/rhash_timer.c |  98 ++++++++++++
 .../selftests/bpf/progs/timer_mim_reject.c    |  84 ++++++++++-
 9 files changed, 525 insertions(+), 21 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhash_timer.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhash_timer.c


base-commit: 254c881fe0554c5efb16d355c273702a27a32a20
-- 
2.53.0

[PATCH bpf v1 1/8] bpf: Preserve special fields in recycled rhtab elements

[Kumar Kartikeya Dwivedi]

From: Yuan Chen <chenyuan@kylinos.cn>

rhtab_map_update_elem() initializes special fields after obtaining an
element from bpf_mem_cache_alloc(). The allocator can return a fresh,
zeroed unit, or recycle one from its RCU-pending lists before the
registered destructor has run.

A BPF program can retain a map-value pointer after deleting its element
and initialize and arm a timer through that pointer. If the deleted unit
is recycled, check_and_init_map_value() clears the only pointer to the
timer. Neither a later deletion nor rhtab_mem_dtor() can then cancel it,
and the callback can run with its key and value pointing into freed memory.

Do not reinitialize special fields on insertion. Fresh allocator units are
already zeroed. For recycled units, the special fields are ownership state
that must remain visible to the eventual destructor. copy_map_value()
already skips those fields, matching the non-preallocated hash-map path and
the lifecycle established by commit 275c30bcee66 ("bpf: Don't reinit map
value in prealloc_lru_pop").

Fixes: 6905f8601298 ("bpf: Allow special fields in resizable hashtab")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
[ kkd: Split out the fix and rewrote the commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/hashtab.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index e89fde188389..527cc5716ee8 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -3070,7 +3070,6 @@ static long rhtab_map_update_elem(struct bpf_map *map, void *key, void *value, u
 
 	memcpy(elem->data, key, map->key_size);
 	copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
-	check_and_init_map_value(map, rhtab_elem_value(elem, map->key_size));
 
 	/* Prevent deadlock for NMI programs attempting to take bucket lock */
 	bpf_disable_instrumentation();
-- 
2.53.0

[PATCH bpf v1 2/8] selftests/bpf: Test timer field on recycled rhtab element

[Kumar Kartikeya Dwivedi]

Exercise the rhtab special-field lifecycle with the sequence from the
original report. A bpf_for_each_map_elem() callback deletes the sole
element, then initializes and arms a timer through the callback value
pointer while it remains valid.

Use a one-element map and pin userspace and BPF execution to one CPU.
Repeated delete-and-replace cycles drain the per-CPU allocator cache, and
periodic RCU synchronization makes the deleted units available for
recycling.

After each replacement, a second BPF program calls bpf_timer_cancel()
on its value. A successful cancellation proves both that a timer-bearing
unit was recycled and that insertion preserved the timer field. Without
the fix, insertion clears that field and cancellation keeps returning
-EINVAL. A long expiration keeps the timer callback out of the test, so
the regression is detected without accessing freed memory.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/rhash_timer.c    | 141 ++++++++++++++++++
 .../testing/selftests/bpf/progs/rhash_timer.c |  98 ++++++++++++
 2 files changed, 239 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/rhash_timer.c
 create mode 100644 tools/testing/selftests/bpf/progs/rhash_timer.c

diff --git a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
new file mode 100644
index 000000000000..3aad9fc02e06
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <test_progs.h>
+#include "rhash_timer.skel.h"
+
+#define MAX_ATTEMPTS 256
+#define RCU_SYNC_INTERVAL 64
+
+static int pin_to_first_cpu(cpu_set_t *old_mask)
+{
+	cpu_set_t new_mask;
+	int cpu;
+
+	if (sched_getaffinity(0, sizeof(*old_mask), old_mask))
+		return -errno;
+
+	for (cpu = 0; cpu < CPU_SETSIZE; cpu++)
+		if (CPU_ISSET(cpu, old_mask))
+			break;
+	if (cpu == CPU_SETSIZE)
+		return -EINVAL;
+
+	CPU_ZERO(&new_mask);
+	CPU_SET(cpu, &new_mask);
+	if (sched_setaffinity(0, sizeof(new_mask), &new_mask))
+		return -errno;
+	return 0;
+}
+
+static int update_timer_map(int map_fd, __u64 key)
+{
+	__u64 value[3] = {};
+
+	return bpf_map_update_elem(map_fd, &key, value, BPF_NOEXIST);
+}
+
+static int run_prog(int prog_fd, struct bpf_test_run_opts *opts)
+{
+	int err;
+
+	err = bpf_prog_test_run_opts(prog_fd, opts);
+	if (err)
+		return err;
+	return opts->retval;
+}
+
+void test_rhash_timer(void)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+	struct rhash_timer *skel = NULL;
+	cpu_set_t old_mask;
+	int map_fd = -1, arm_fd, cancel_fd;
+	bool affinity_set = false;
+	__u64 key = 1;
+	int attempt, err;
+
+	err = pin_to_first_cpu(&old_mask);
+	if (!ASSERT_OK(err, "pin_to_first_cpu"))
+		return;
+	affinity_set = true;
+
+	skel = rhash_timer__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		goto out;
+
+	map_fd = bpf_map__fd(skel->maps.timer_map);
+	if (!ASSERT_GE(map_fd, 0, "timer_map fd"))
+		goto out;
+	arm_fd = bpf_program__fd(skel->progs.arm_deleted_timer);
+	if (!ASSERT_GE(arm_fd, 0, "arm_deleted_timer fd"))
+		goto out;
+	cancel_fd = bpf_program__fd(skel->progs.cancel_recycled_timer);
+	if (!ASSERT_GE(cancel_fd, 0, "cancel_recycled_timer fd"))
+		goto out;
+
+	err = update_timer_map(map_fd, key);
+	if (!ASSERT_OK(err, "seed_timer_map"))
+		goto out;
+
+	for (attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
+		err = run_prog(arm_fd, &opts);
+		if (err) {
+			ASSERT_OK(err, "arm_deleted_timer");
+			goto out;
+		}
+		if (skel->bss->armed != attempt + 1) {
+			ASSERT_EQ(skel->bss->armed, attempt + 1, "armed");
+			goto out;
+		}
+		if (skel->bss->timer_init_err) {
+			ASSERT_OK(skel->bss->timer_init_err, "timer_init_err");
+			goto out;
+		}
+		if (skel->bss->timer_set_callback_err) {
+			ASSERT_OK(skel->bss->timer_set_callback_err,
+				  "timer_set_callback_err");
+			goto out;
+		}
+		if (skel->bss->timer_start_err) {
+			ASSERT_OK(skel->bss->timer_start_err, "timer_start_err");
+			goto out;
+		}
+
+		if ((attempt + 1) % RCU_SYNC_INTERVAL == 0) {
+			err = kern_sync_rcu();
+			if (err) {
+				ASSERT_OK(err, "kern_sync_rcu");
+				goto out;
+			}
+		}
+
+		err = update_timer_map(map_fd, ++key);
+		if (err) {
+			ASSERT_OK(err, "replace_timer_map");
+			goto out;
+		}
+
+		err = run_prog(cancel_fd, &opts);
+		if (err) {
+			ASSERT_OK(err, "cancel_recycled_timer");
+			goto out;
+		}
+		if (skel->bss->timer_cancel_err) {
+			ASSERT_OK(skel->bss->timer_cancel_err, "timer_cancel_err");
+			goto out;
+		}
+		if (skel->bss->cancelled)
+			break;
+	}
+
+	ASSERT_GT(skel->bss->cancelled, 0, "preserved timer");
+out:
+	if (map_fd >= 0)
+		bpf_map_delete_elem(map_fd, &key);
+	rhash_timer__destroy(skel);
+	if (affinity_set)
+		sched_setaffinity(0, sizeof(old_mask), &old_mask);
+}
diff --git a/tools/testing/selftests/bpf/progs/rhash_timer.c b/tools/testing/selftests/bpf/progs/rhash_timer.c
new file mode 100644
index 000000000000..2e06a463c605
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/rhash_timer.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <vmlinux.h>
+#include <errno.h>
+#include <bpf/bpf_helpers.h>
+
+#define CLOCK_MONOTONIC 1
+#define TIMER_NSEC (60ULL * 1000 * 1000 * 1000)
+
+struct timer_value {
+	struct bpf_timer timer;
+	u64 data;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__uint(max_entries, 1);
+	__type(key, u64);
+	__type(value, struct timer_value);
+} timer_map SEC(".maps");
+
+u64 armed;
+u64 cancelled;
+long timer_init_err;
+long timer_set_callback_err;
+long timer_start_err;
+long timer_cancel_err;
+
+static int timer_cb(void *map, u64 *key, struct timer_value *value)
+{
+	return 0;
+}
+
+static long arm_timer_cb(struct bpf_map *map, u64 *key,
+			 struct timer_value *value, void *ctx)
+{
+	u64 key_copy = *key;
+	long err;
+
+	err = bpf_map_delete_elem(map, &key_copy);
+	if (err)
+		return 1;
+
+	err = bpf_timer_init(&value->timer, map, CLOCK_MONOTONIC);
+	if (err) {
+		timer_init_err = err;
+		return 1;
+	}
+
+	err = bpf_timer_set_callback(&value->timer, timer_cb);
+	if (err) {
+		timer_set_callback_err = err;
+		return 1;
+	}
+
+	err = bpf_timer_start(&value->timer, TIMER_NSEC, BPF_F_TIMER_CPU_PIN);
+	if (err) {
+		timer_start_err = err;
+		return 1;
+	}
+
+	__sync_fetch_and_add(&armed, 1);
+	return 1;
+}
+
+static long cancel_timer_cb(struct bpf_map *map, u64 *key,
+			    struct timer_value *value, void *ctx)
+{
+	long err;
+
+	err = bpf_timer_cancel(&value->timer);
+	if (err == -EINVAL)
+		return 1;
+	if (err < 0) {
+		timer_cancel_err = err;
+		return 1;
+	}
+
+	__sync_fetch_and_add(&cancelled, 1);
+	return 1;
+}
+
+SEC("syscall")
+int arm_deleted_timer(void *ctx)
+{
+	bpf_for_each_map_elem(&timer_map, arm_timer_cb, NULL, 0);
+	return 0;
+}
+
+SEC("syscall")
+int cancel_recycled_timer(void *ctx)
+{
+	bpf_for_each_map_elem(&timer_map, cancel_timer_cb, NULL, 0);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.53.0

[PATCH bpf v1 3/8] bpf: Cancel special fields when recycling rhtab elements

[Kumar Kartikeya Dwivedi]

From: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

rhtab_map_update_existing() and rhtab_delete_elem() call
bpf_obj_free_fields() when replacing or deleting a value. These map
operations can run from BPF programs in NMI context, where releasing a
referenced kptr or another complex field is not generally safe.

Array and hash maps avoid that problem by cancelling only the asynchronous
fields which can be stopped safely in the caller context. Other ownership
state remains attached to the allocation until its memory allocator
destructor performs the final cleanup.

Use bpf_obj_cancel_fields() for the corresponding rhtab paths as well. This
cancels timers, workqueues, and task work while allowing rhtab_mem_dtor() to
release referenced kptrs when the allocation is eventually destroyed.

Fixes: 6905f8601298 ("bpf: Allow special fields in resizable hashtab")
Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>
[ kkd: Rebased, used direct helper calls, and rewrote the commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/hashtab.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 527cc5716ee8..cc60e99ffbe9 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2868,16 +2868,6 @@ static int rhtab_map_alloc_check(union bpf_attr *attr)
 	return htab_map_alloc_check(attr);
 }
 
-static void rhtab_check_and_free_fields(struct bpf_rhtab *rhtab,
-					struct rhtab_elem *elem)
-{
-	if (IS_ERR_OR_NULL(rhtab->map.record))
-		return;
-
-	bpf_obj_free_fields(rhtab->map.record,
-			    rhtab_elem_value(elem, rhtab->map.key_size));
-}
-
 static void rhtab_mem_dtor(void *obj, void *ctx)
 {
 	struct htab_btf_record *hrec = ctx;
@@ -2967,8 +2957,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
 		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
 		check_and_init_map_value(&rhtab->map, copy);
 	}
-	/* Release internal structs: kptr, bpf_timer, task_work, wq */
-	rhtab_check_and_free_fields(rhtab, elem);
+	bpf_obj_cancel_fields(&rhtab->map,
+			      rhtab_elem_value(elem, rhtab->map.key_size));
 	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
 	return 0;
 }
@@ -3009,7 +2999,6 @@ static int rhtab_map_lookup_and_delete_elem(struct bpf_map *map, void *key, void
 static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *elem, void *value,
 				      u64 map_flags)
 {
-	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
 	void *old_val = rhtab_elem_value(elem, map->key_size);
 
 	if (map_flags & BPF_NOEXIST)
@@ -3029,7 +3018,7 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
 	 * kptrs/etc. still sit in the slot. Cancel them after the copy
 	 * to match arraymap's update semantics.
 	 */
-	rhtab_check_and_free_fields(rhtab, elem);
+	bpf_obj_cancel_fields(map, old_val);
 	return 0;
 }
 
-- 
2.53.0

[PATCH bpf v1 4/8] selftests/bpf: Test rhtab kptr cancellation semantics

[Kumar Kartikeya Dwivedi]

From: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

Resizable hash-map updates and deletions must not perform full special-field
destruction in their caller context. In particular, a referenced kptr must
remain attached to the allocation until the memory allocator destructor can
release it safely.

Add separate coverage for both affected paths. The update test stores a task
kptr, replaces the ordinary value bytes with BPF_EXIST, and verifies that the
kptr survived. The delete test removes an element and exchanges its kptr
through the still-valid map-value pointer before the allocation is reclaimed.

Both cases observe a NULL kptr when rhtab uses bpf_obj_free_fields(). They
recover and release the reference after rhtab switches to cancellation
semantics.

Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
[ kkd: Split update and delete coverage and rewrote the commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../testing/selftests/bpf/prog_tests/rhash.c  |   6 +
 tools/testing/selftests/bpf/progs/rhash.c     | 112 ++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/rhash.c b/tools/testing/selftests/bpf/prog_tests/rhash.c
index 98bb66907b7f..0641bd5b0a9e 100644
--- a/tools/testing/selftests/bpf/prog_tests/rhash.c
+++ b/tools/testing/selftests/bpf/prog_tests/rhash.c
@@ -172,6 +172,12 @@ void test_rhash(void)
 	if (test__start_subtest("test_rhash_delete_nonexistent"))
 		rhash_run("test_rhash_delete_nonexistent");
 
+	if (test__start_subtest("test_rhash_kptr_update"))
+		rhash_run("test_rhash_kptr_update");
+
+	if (test__start_subtest("test_rhash_kptr_delete"))
+		rhash_run("test_rhash_kptr_delete");
+
 	if (test__start_subtest("test_rhash_map_extra_presize"))
 		rhash_map_extra_presize();
 
diff --git a/tools/testing/selftests/bpf/progs/rhash.c b/tools/testing/selftests/bpf/progs/rhash.c
index fc2dac3a719e..aea4de8dc781 100644
--- a/tools/testing/selftests/bpf/progs/rhash.c
+++ b/tools/testing/selftests/bpf/progs/rhash.c
@@ -19,6 +19,11 @@ struct elem {
 	int val;
 };
 
+struct special_elem {
+	struct task_struct __kptr *task;
+	int val;
+};
+
 struct {
 	__uint(type, BPF_MAP_TYPE_RHASH);
 	__uint(map_flags, BPF_F_NO_PREALLOC);
@@ -27,6 +32,17 @@ struct {
 	__type(value, struct elem);
 } rhmap SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_RHASH);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct special_elem);
+} special_fields SEC(".maps");
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+extern void bpf_task_release(struct task_struct *p) __ksym;
+
 SEC("syscall")
 int test_rhash_lookup_update(void *ctx)
 {
@@ -246,3 +262,99 @@ int test_rhash_delete_nonexistent(void *ctx)
 	err = 0;
 	return 0;
 }
+
+SEC("syscall")
+int test_rhash_kptr_update(void *ctx)
+{
+	struct special_elem val1 = { .val = 1 };
+	struct special_elem val2 = { .val = 2 };
+	struct task_struct *task, *old;
+	struct special_elem *elem;
+	int key = 0;
+
+	err = 1;
+	if (bpf_map_update_elem(&special_fields, &key, &val1, BPF_NOEXIST))
+		return 1;
+
+	err = 2;
+	elem = bpf_map_lookup_elem(&special_fields, &key);
+	if (!elem)
+		return 2;
+
+	err = 3;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 3;
+
+	err = 4;
+	old = bpf_kptr_xchg(&elem->task, task);
+	if (old) {
+		bpf_task_release(old);
+		return 4;
+	}
+
+	err = 5;
+	if (bpf_map_update_elem(&special_fields, &key, &val2, BPF_EXIST))
+		return 5;
+
+	err = 6;
+	elem = bpf_map_lookup_elem(&special_fields, &key);
+	if (!elem || elem->val != 2)
+		return 6;
+
+	err = 7;
+	old = bpf_kptr_xchg(&elem->task, NULL);
+	if (!old)
+		return 7;
+	bpf_task_release(old);
+
+	err = 8;
+	if (bpf_map_delete_elem(&special_fields, &key))
+		return 8;
+
+	err = 0;
+	return 0;
+}
+
+SEC("syscall")
+int test_rhash_kptr_delete(void *ctx)
+{
+	struct special_elem val = {};
+	struct task_struct *task, *old;
+	struct special_elem *elem;
+	int key = 0;
+
+	err = 1;
+	if (bpf_map_update_elem(&special_fields, &key, &val, BPF_NOEXIST))
+		return 1;
+
+	err = 2;
+	elem = bpf_map_lookup_elem(&special_fields, &key);
+	if (!elem)
+		return 2;
+
+	err = 3;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 3;
+
+	err = 4;
+	old = bpf_kptr_xchg(&elem->task, task);
+	if (old) {
+		bpf_task_release(old);
+		return 4;
+	}
+
+	err = 5;
+	if (bpf_map_delete_elem(&special_fields, &key))
+		return 5;
+
+	err = 6;
+	old = bpf_kptr_xchg(&elem->task, NULL);
+	if (!old)
+		return 6;
+	bpf_task_release(old);
+
+	err = 0;
+	return 0;
+}
-- 
2.53.0

[PATCH bpf v1 5/8] bpf: Mark NULL kptr stores precise

[Kumar Kartikeya Dwivedi]

check_map_kptr_access() permits a scalar store into an untrusted kptr
field only when the register is known to contain zero. Unlike other
verifier checks whose outcome depends on a scalar value, it does not mark
that register precise.

A state checkpoint reached with an imprecise zero can therefore prune a
second path that reaches the store with an arbitrary nonzero scalar. The
program can write attacker-controlled bits into the kptr field and load
them back as a PTR_TO_BTF_ID.

Call mark_chain_precision() before accepting a known-zero register. This
forces state equivalence to compare its scalar range and makes the verifier
visit and reject a path carrying a nonzero value.

Fixes: 61df10c7799e ("bpf: Allow storing unreferenced kptr in map")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d7dd0befbd10..ba9753bf529a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4694,8 +4694,15 @@ static int check_map_kptr_access(struct bpf_verifier_env *env,
 			return ret;
 	} else if (class == BPF_STX) {
 		val_reg = reg_state(env, value_regno);
-		if (!bpf_register_is_null(val_reg) &&
-		    map_kptr_match_type(env, kptr_field, val_reg, value_regno))
+		if (bpf_register_is_null(val_reg)) {
+			/*
+			 * This store is valid only because the scalar is known to be
+			 * zero. Mark it precise so another scalar cannot be pruned
+			 * against this state.
+			 */
+			return mark_chain_precision(env, value_regno);
+		}
+		if (map_kptr_match_type(env, kptr_field, val_reg, value_regno))
 			return -EACCES;
 	} else if (class == BPF_ST) {
 		if (insn->imm) {
-- 
2.53.0

[PATCH bpf v1 6/8] selftests/bpf: Test imprecise scalar kptr stores

[Kumar Kartikeya Dwivedi]

Add a verifier regression where an imprecise zero scalar reaches a kptr
store first and a nonzero scalar reaches the same instruction on a second
path.

Without the corresponding verifier fix, the second path is pruned and the
program is unexpectedly accepted. With the fix, the scalar range is
compared and the invalid store is rejected.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/map_kptr_fail.c       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index 5e25ca806060..eee35d203b66 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -409,4 +409,41 @@ int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
 	return 0;
 }
 
+SEC("?tc")
+__description("reject imprecise scalar store to kptr after state pruning")
+__failure __msg("invalid kptr access, R7 type=scalar")
+__naked void reject_imprecise_scalar_store_to_kptr(void)
+{
+	asm volatile (
+		"r0 = 0;"
+		"*(u32 *)(r10 - 4) = r0;"
+		"r2 = r10;"
+		"r2 += -4;"
+		"r1 = %[array_map] ll;"
+		"call %[bpf_map_lookup_elem];"
+		"if r0 == 0 goto l2_%=;"
+		"r6 = r0;"
+		"r9 = *(u64 *)(r6 + 0);"
+		"if r9 != 0 goto l0_%=;"
+		"r7 = 0;"
+		".rept 10;"
+		"r5 = 1;"
+		".endr;"
+		"goto l1_%=;"
+	"l0_%=:"
+		"r7 = 0x4141414141414141 ll;"
+		".rept 10;"
+		"r5 = 1;"
+		".endr;"
+	"l1_%=:"
+		"*(u64 *)(r6 + 8) = r7;"
+	"l2_%=:"
+		"r0 = 0;"
+		"exit;"
+		:
+		: __imm(bpf_map_lookup_elem),
+		  __imm_addr(array_map)
+		: __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.53.0

[PATCH bpf v1 7/8] bpf: Preserve inner map identity in callback frames

[Kumar Kartikeya Dwivedi]

Callback frame constructors initialize map-typed argument registers with
__mark_reg_known_zero() and then restore map_ptr. This clears map_uid,
which is the only field distinguishing inner maps that share an
inner_map_meta template.

When a timer callback invokes bpf_for_each_map_elem() on a second inner
map, both the saved first map and the second map value can reach the nested
callback as the same template with map_uid zero. bpf_timer_init() then
accepts pairing the timer from the second map with the first map.

The runtime records the first map in the timer without taking a reference.
Freeing that map does not find the timer stored in the second map, so a
later timer callback dereferences the freed map.

Copy map_uid from the same caller register as map_ptr when constructing
for-each, timer/workqueue, and task-work callback arguments. The existing
identity check can then reject mismatched inner maps while allowing a
callback value to be paired with its actual map.

Fixes: 3e8ce29850f1 ("bpf: Prevent pointer mismatch in bpf_timer_init.")
Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
Fixes: 5c8fd7e2b5b0 ("bpf: bpf task work plumbing")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ba9753bf529a..c9c0504b7edd 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9994,10 +9994,12 @@ int map_set_for_each_callback_args(struct bpf_verifier_env *env,
 	callee->regs[BPF_REG_2].type = PTR_TO_MAP_KEY;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_2]);
 	callee->regs[BPF_REG_2].map_ptr = caller->regs[BPF_REG_1].map_ptr;
+	callee->regs[BPF_REG_2].map_uid = caller->regs[BPF_REG_1].map_uid;
 
 	callee->regs[BPF_REG_3].type = PTR_TO_MAP_VALUE;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = caller->regs[BPF_REG_1].map_ptr;
+	callee->regs[BPF_REG_3].map_uid = caller->regs[BPF_REG_1].map_uid;
 
 	/* pointer to stack or null */
 	callee->regs[BPF_REG_4] = caller->regs[BPF_REG_3];
@@ -10075,6 +10077,7 @@ static int set_timer_callback_state(struct bpf_verifier_env *env,
 				    int insn_idx)
 {
 	struct bpf_map *map_ptr = caller->regs[BPF_REG_1].map_ptr;
+	u32 map_uid = caller->regs[BPF_REG_1].map_uid;
 
 	/* bpf_timer_set_callback(struct bpf_timer *timer, void *callback_fn);
 	 * callback_fn(struct bpf_map *map, void *key, void *value);
@@ -10082,14 +10085,17 @@ static int set_timer_callback_state(struct bpf_verifier_env *env,
 	callee->regs[BPF_REG_1].type = CONST_PTR_TO_MAP;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_1]);
 	callee->regs[BPF_REG_1].map_ptr = map_ptr;
+	callee->regs[BPF_REG_1].map_uid = map_uid;
 
 	callee->regs[BPF_REG_2].type = PTR_TO_MAP_KEY;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_2]);
 	callee->regs[BPF_REG_2].map_ptr = map_ptr;
+	callee->regs[BPF_REG_2].map_uid = map_uid;
 
 	callee->regs[BPF_REG_3].type = PTR_TO_MAP_VALUE;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
+	callee->regs[BPF_REG_3].map_uid = map_uid;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
@@ -10189,6 +10195,7 @@ static int set_task_work_schedule_callback_state(struct bpf_verifier_env *env,
 						 int insn_idx)
 {
 	struct bpf_map *map_ptr = caller->regs[BPF_REG_3].map_ptr;
+	u32 map_uid = caller->regs[BPF_REG_3].map_uid;
 
 	/*
 	 * callback_fn(struct bpf_map *map, void *key, void *value);
@@ -10196,14 +10203,17 @@ static int set_task_work_schedule_callback_state(struct bpf_verifier_env *env,
 	callee->regs[BPF_REG_1].type = CONST_PTR_TO_MAP;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_1]);
 	callee->regs[BPF_REG_1].map_ptr = map_ptr;
+	callee->regs[BPF_REG_1].map_uid = map_uid;
 
 	callee->regs[BPF_REG_2].type = PTR_TO_MAP_KEY;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_2]);
 	callee->regs[BPF_REG_2].map_ptr = map_ptr;
+	callee->regs[BPF_REG_2].map_uid = map_uid;
 
 	callee->regs[BPF_REG_3].type = PTR_TO_MAP_VALUE;
 	__mark_reg_known_zero(&callee->regs[BPF_REG_3]);
 	callee->regs[BPF_REG_3].map_ptr = map_ptr;
+	callee->regs[BPF_REG_3].map_uid = map_uid;
 
 	/* unused */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
-- 
2.53.0

[PATCH bpf v1 8/8] selftests/bpf: Test inner map identities in callbacks

[Kumar Kartikeya Dwivedi]

Add load-only timer_mim coverage for inner map identities propagated
through nested timer and bpf_for_each_map_elem() callbacks.

The negative case initializes a timer in the second inner map with the map
saved from the first inner map timer callback. The positive case pairs the
timer value with the map supplied to the same for-each callback.

Without the verifier fix, the mismatched-map program is accepted while the
same-map control is rejected. Preserving map_uid reverses both verdicts.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/timer_mim.c      | 29 ++++++-
 .../selftests/bpf/progs/timer_mim_reject.c    | 84 ++++++++++++++++++-
 2 files changed, 109 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/timer_mim.c b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
index c930c7d7105b..fa7bb769ca31 100644
--- a/tools/testing/selftests/bpf/prog_tests/timer_mim.c
+++ b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
@@ -59,10 +59,32 @@ void serial_test_timer_mim(void)
 	int err;
 
 	old_print_fn = libbpf_set_print(NULL);
-	timer_reject_skel = timer_mim_reject__open_and_load();
-	libbpf_set_print(old_print_fn);
-	if (!ASSERT_ERR_PTR(timer_reject_skel, "timer_reject_skel_load"))
+	timer_reject_skel = timer_mim_reject__open();
+	if (!ASSERT_OK_PTR(timer_reject_skel, "timer_reject_skel_open"))
+		goto cleanup;
+	bpf_program__set_autoload(timer_reject_skel->progs.test1, true);
+	err = timer_mim_reject__load(timer_reject_skel);
+	ASSERT_ERR(err, "timer_reject_skel_load");
+	timer_mim_reject__destroy(timer_reject_skel);
+
+	timer_reject_skel = timer_mim_reject__open();
+	if (!ASSERT_OK_PTR(timer_reject_skel, "callback_reject_skel_open"))
 		goto cleanup;
+	bpf_program__set_autoload(timer_reject_skel->progs.callback_map_uid_mismatch, true);
+	err = timer_mim_reject__load(timer_reject_skel);
+	ASSERT_ERR(err, "callback_reject_skel_load");
+	timer_mim_reject__destroy(timer_reject_skel);
+
+	timer_reject_skel = timer_mim_reject__open();
+	if (!ASSERT_OK_PTR(timer_reject_skel, "callback_accept_skel_open"))
+		goto cleanup;
+	bpf_program__set_autoload(timer_reject_skel->progs.callback_map_uid_match, true);
+	err = timer_mim_reject__load(timer_reject_skel);
+	if (!ASSERT_OK(err, "callback_accept_skel_load"))
+		goto cleanup;
+	timer_mim_reject__destroy(timer_reject_skel);
+	timer_reject_skel = NULL;
+	libbpf_set_print(old_print_fn);
 
 	timer_skel = timer_mim__open_and_load();
 	if (!timer_skel && errno == EOPNOTSUPP) {
@@ -75,6 +97,7 @@ void serial_test_timer_mim(void)
 	err = timer_mim(timer_skel);
 	ASSERT_OK(err, "timer_mim");
 cleanup:
+	libbpf_set_print(old_print_fn);
 	timer_mim__destroy(timer_skel);
 	timer_mim_reject__destroy(timer_reject_skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/timer_mim_reject.c b/tools/testing/selftests/bpf/progs/timer_mim_reject.c
index dd3f1ed6d6e6..83f31138336b 100644
--- a/tools/testing/selftests/bpf/progs/timer_mim_reject.c
+++ b/tools/testing/selftests/bpf/progs/timer_mim_reject.c
@@ -43,7 +43,7 @@ static int timer_cb(void *map, int *key, struct hmap_elem *val)
 	return 0;
 }
 
-SEC("fentry/bpf_fentry_test1")
+SEC("?fentry/bpf_fentry_test1")
 int BPF_PROG(test1, int a)
 {
 	struct hmap_elem init = {};
@@ -72,3 +72,85 @@ int BPF_PROG(test1, int a)
 		err |= 8;
 	return 0;
 }
+
+struct callback_ctx {
+	void *map;
+};
+
+static int mismatch_iter_cb(void *map, int *key, struct hmap_elem *val, struct callback_ctx *ctx)
+{
+	bpf_timer_init(&val->timer, ctx->map, CLOCK_MONOTONIC);
+	return 0;
+}
+
+static int timer_mismatch_cb(void *map, int *key, struct hmap_elem *val)
+{
+	struct callback_ctx ctx = { .map = map };
+	struct bpf_map *inner_map2;
+	int array_key2 = ARRAY_KEY2;
+
+	inner_map2 = bpf_map_lookup_elem(&outer_arr, &array_key2);
+	if (!inner_map2)
+		return 0;
+	bpf_for_each_map_elem(inner_map2, mismatch_iter_cb, &ctx, 0);
+	return 0;
+}
+
+static int match_iter_cb(void *map, int *key, struct hmap_elem *val, struct callback_ctx *ctx)
+{
+	bpf_timer_init(&val->timer, map, CLOCK_MONOTONIC);
+	return 0;
+}
+
+static int timer_match_cb(void *map, int *key, struct hmap_elem *val)
+{
+	struct callback_ctx ctx = {};
+	struct bpf_map *inner_map2;
+	int array_key2 = ARRAY_KEY2;
+
+	inner_map2 = bpf_map_lookup_elem(&outer_arr, &array_key2);
+	if (!inner_map2)
+		return 0;
+	bpf_for_each_map_elem(inner_map2, match_iter_cb, &ctx, 0);
+	return 0;
+}
+
+SEC("?fentry/bpf_fentry_test1")
+int BPF_PROG(callback_map_uid_mismatch, int a)
+{
+	struct hmap_elem *val;
+	struct bpf_map *inner_map;
+	int array_key = ARRAY_KEY;
+	int hash_key = HASH_KEY;
+
+	inner_map = bpf_map_lookup_elem(&outer_arr, &array_key);
+	if (!inner_map)
+		return 0;
+	val = bpf_map_lookup_elem(inner_map, &hash_key);
+	if (!val)
+		return 0;
+
+	bpf_timer_init(&val->timer, inner_map, CLOCK_MONOTONIC);
+	bpf_timer_set_callback(&val->timer, timer_mismatch_cb);
+	return 0;
+}
+
+SEC("?fentry/bpf_fentry_test1")
+int BPF_PROG(callback_map_uid_match, int a)
+{
+	struct hmap_elem *val;
+	struct bpf_map *inner_map;
+	int array_key = ARRAY_KEY;
+	int hash_key = HASH_KEY;
+
+	inner_map = bpf_map_lookup_elem(&outer_arr, &array_key);
+	if (!inner_map)
+		return 0;
+	val = bpf_map_lookup_elem(inner_map, &hash_key);
+	if (!val)
+		return 0;
+
+	bpf_timer_init(&val->timer, inner_map, CLOCK_MONOTONIC);
+	bpf_timer_set_callback(&val->timer, timer_match_cb);
+	return 0;
+}
-- 
2.53.0

Re: [PATCH bpf v1 2/8] selftests/bpf: Test timer field on recycled rhtab element

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant file descriptor validation after successful BPF skeleton open_and_load
--

commit 3c5ef82656b9b8e3dd5798348adbe7dae8162f8d
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Test timer field on recycled rhtab element

This commit adds a selftest to exercise the rhtab special-field lifecycle
using a one-element map. It verifies that deleting a map element and then
replacing it preserves the timer field and successfully recycles the element.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
> new file mode 100644
> index 0000000000000..3aad9fc02e063
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c

[ ... ]

> +void test_rhash_timer(void)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct rhash_timer *skel = NULL;

[ ... ]

> +	skel = rhash_timer__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		goto out;
> +
> +	map_fd = bpf_map__fd(skel->maps.timer_map);
> +	if (!ASSERT_GE(map_fd, 0, "timer_map fd"))
> +		goto out;

[Severity: Low]
According to the BPF subsystem guidelines for the skeleton API, after a
successful rhash_timer__open_and_load() call, the skeleton pointer is valid
and all maps are guaranteed to be created with valid FDs. Is it necessary
to manually extract and validate map_fd using ASSERT_GE() here?

> +	arm_fd = bpf_program__fd(skel->progs.arm_deleted_timer);
> +	if (!ASSERT_GE(arm_fd, 0, "arm_deleted_timer fd"))
> +		goto out;
> +	cancel_fd = bpf_program__fd(skel->progs.cancel_recycled_timer);
> +	if (!ASSERT_GE(cancel_fd, 0, "cancel_recycled_timer fd"))
> +		goto out;

[Severity: Low]
Similarly, is this program FD validation needed? The skeleton guarantees
that all programs are loaded with valid FDs after open_and_load succeeds,
so bpf_program__fd() cannot return a negative value here.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904104203.345917-1-memxor@gmail.com?part=2

Re: [PATCH bpf v1 3/8] bpf: Cancel special fields when recycling rhtab elements

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing bpf_obj_free_fields() with bpf_obj_cancel_fields() during map updates and deletions violates BTF-001 and causes cross-key resource leakage.
--

commit 87b60bb947c49832ff2d2552730fd3c5124e6130
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

bpf: Cancel special fields when recycling rhtab elements

This commit modifies rhtab map operations to use bpf_obj_cancel_fields()
instead of bpf_obj_free_fields() when replacing or deleting elements. This is
done to avoid releasing referenced kptrs and other complex fields in NMI
context, aligning with array and hash map behavior.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 527cc5716ee8c..cc60e99ffbe92 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> @@ -2967,8 +2957,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>  		check_and_init_map_value(&rhtab->map, copy);
>  	}
> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);

[Severity: High]
Does changing this to bpf_obj_cancel_fields() leave stale special fields in
the memory cache?

Since bpf_obj_cancel_fields() does not drop kptrs or list heads, these
references appear to remain in the element when it is freed to the cache.
When a new element is later allocated from the cache in
rhtab_map_update_elem():

    elem = bpf_mem_cache_alloc(&rhtab->ma);
    if (!elem)
        return -ENOMEM;
    memcpy(elem->data, key, map->key_size);
    copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);

Because bpf_mem_cache_alloc() does not zero memory and copy_map_value()
skips special fields, will this cause the new key to inherit the old key's
populated list head or kptr?

>  	return 0;
>  }

[ ... ]

> @@ -3029,7 +3018,7 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * kptrs/etc. still sit in the slot. Cancel them after the copy
>  	 * to match arraymap's update semantics.
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(map, old_val);

[Severity: High]
Does replacing the free operation with bpf_obj_cancel_fields() here violate
the requirement to clean up special BTF fields during map updates?

When overwriting an existing map value in-place, failing to free existing
kptrs or list heads seems like it causes them to outlive the update. Will
this leave stale resources bound to the key indefinitely?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904104203.345917-1-memxor@gmail.com?part=3

Re: [PATCH bpf v1 3/8] bpf: Cancel special fields when recycling rhtab elements

[Kumar Kartikeya Dwivedi]

On Fri Sep 4, 2026 at 1:37 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Replacing bpf_obj_free_fields() with bpf_obj_cancel_fields() during map updates and deletions violates BTF-001 and causes cross-key resource leakage.
> --
>
> commit 87b60bb947c49832ff2d2552730fd3c5124e6130
> Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
>
> bpf: Cancel special fields when recycling rhtab elements
>
> This commit modifies rhtab map operations to use bpf_obj_cancel_fields()
> instead of bpf_obj_free_fields() when replacing or deleting elements. This is
> done to avoid releasing referenced kptrs and other complex fields in NMI
> context, aligning with array and hash map behavior.
>
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 527cc5716ee8c..cc60e99ffbe92 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>
> [ ... ]
>
>> @@ -2967,8 +2957,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>>  		check_and_init_map_value(&rhtab->map, copy);
>>  	}
>> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
>> -	rhtab_check_and_free_fields(rhtab, elem);
>> +	bpf_obj_cancel_fields(&rhtab->map,
>> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>
> [Severity: High]
> Does changing this to bpf_obj_cancel_fields() leave stale special fields in
> the memory cache?
>
> Since bpf_obj_cancel_fields() does not drop kptrs or list heads, these
> references appear to remain in the element when it is freed to the cache.
> When a new element is later allocated from the cache in
> rhtab_map_update_elem():
>
>     elem = bpf_mem_cache_alloc(&rhtab->ma);
>     if (!elem)
>         return -ENOMEM;
>     memcpy(elem->data, key, map->key_size);
>     copy_map_value(map, rhtab_elem_value(elem, map->key_size), value);
>
> Because bpf_mem_cache_alloc() does not zero memory and copy_map_value()
> skips special fields, will this cause the new key to inherit the old key's
> populated list head or kptr?
>
>>  	return 0;
>>  }
>
> [ ... ]
>
>> @@ -3029,7 +3018,7 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>>  	 * kptrs/etc. still sit in the slot. Cancel them after the copy
>>  	 * to match arraymap's update semantics.
>>  	 */
>> -	rhtab_check_and_free_fields(rhtab, elem);
>> +	bpf_obj_cancel_fields(map, old_val);
>
> [Severity: High]
> Does replacing the free operation with bpf_obj_cancel_fields() here violate
> the requirement to clean up special BTF fields during map updates?
>
> When overwriting an existing map value in-place, failing to free existing
> kptrs or list heads seems like it causes them to outlive the update. Will
> this leave stale resources bound to the key indefinitely?
>

Both are deliberate changes.

>>  	return 0;
>>  }

Re: [PATCH bpf v1 6/8] selftests/bpf: Test imprecise scalar kptr stores

[bot+bpf-ci]

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

> selftests/bpf: Test imprecise scalar kptr stores

This isn't a bug, but should this read "Add a verifier regression test"?
As written the first sentence of the commit message says the patch adds a
regression.

> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> index 5e25ca806060c..eee35d203b66f 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> @@ -409,4 +409,41 @@ int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
>  	return 0;
>  }
>
> +SEC("?tc")
> +__description("reject imprecise scalar store to kptr after state pruning")
> +__failure __msg("invalid kptr access, R7 type=scalar")
> +__naked void reject_imprecise_scalar_store_to_kptr(void)
> +{
> +	asm volatile (
> +		"r0 = 0;"
> +		"*(u32 *)(r10 - 4) = r0;"
> +		"r2 = r10;"
> +		"r2 += -4;"
> +		"r1 = %[array_map] ll;"
> +		"call %[bpf_map_lookup_elem];"
> +		"if r0 == 0 goto l2_%=;"
> +		"r6 = r0;"
> +		"r9 = *(u64 *)(r6 + 0);"
> +		"if r9 != 0 goto l0_%=;"
> +		"r7 = 0;"
> +		".rept 10;"
> +		"r5 = 1;"
> +		".endr;"

This isn't a bug, but would a one-line comment above the padding help -
something noting that the ten filler insns are there to get a state
checkpoint recorded at l1_%= (the >=8 insns / >=2 jumps heuristic), so a
future reader knows the test silently loses coverage if that block shrinks?

> +		"goto l1_%=;"
> +	"l0_%=:"
> +		"r7 = 0x4141414141414141 ll;"
> +		".rept 10;"
> +		"r5 = 1;"
> +		".endr;"
> +	"l1_%=:"
> +		"*(u64 *)(r6 + 8) = r7;"
> +	"l2_%=:"
> +		"r0 = 0;"
> +		"exit;"
> +		:
> +		: __imm(bpf_map_lookup_elem),
> +		  __imm_addr(array_map)
> +		: __clobber_all);
> +}
> +
>  char _license[] SEC("license") = "GPL";

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

Re: [PATCH bpf v1 8/8] selftests/bpf: Test inner map identities in callbacks

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/timer_mim.c b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
> index c930c7d7105b9..fa7bb769ca31e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/timer_mim.c
> +++ b/tools/testing/selftests/bpf/prog_tests/timer_mim.c
> @@ -59,10 +59,32 @@ void serial_test_timer_mim(void)
>  	int err;
>
>  	old_print_fn = libbpf_set_print(NULL);
> -	timer_reject_skel = timer_mim_reject__open_and_load();
> -	libbpf_set_print(old_print_fn);
> -	if (!ASSERT_ERR_PTR(timer_reject_skel, "timer_reject_skel_load"))
> +	timer_reject_skel = timer_mim_reject__open();
> +	if (!ASSERT_OK_PTR(timer_reject_skel, "timer_reject_skel_open"))
> +		goto cleanup;
> +	bpf_program__set_autoload(timer_reject_skel->progs.test1, true);
> +	err = timer_mim_reject__load(timer_reject_skel);
> +	ASSERT_ERR(err, "timer_reject_skel_load");
> +	timer_mim_reject__destroy(timer_reject_skel);
> +
> +	timer_reject_skel = timer_mim_reject__open();
> +	if (!ASSERT_OK_PTR(timer_reject_skel, "callback_reject_skel_open"))
>  		goto cleanup;
> +	bpf_program__set_autoload(timer_reject_skel->progs.callback_map_uid_mismatch, true);
> +	err = timer_mim_reject__load(timer_reject_skel);
> +	ASSERT_ERR(err, "callback_reject_skel_load");
> +	timer_mim_reject__destroy(timer_reject_skel);
> +
> +	timer_reject_skel = timer_mim_reject__open();
> +	if (!ASSERT_OK_PTR(timer_reject_skel, "callback_accept_skel_open"))
> +		goto cleanup;
> +	bpf_program__set_autoload(timer_reject_skel->progs.callback_map_uid_match, true);
> +	err = timer_mim_reject__load(timer_reject_skel);
> +	if (!ASSERT_OK(err, "callback_accept_skel_load"))
> +		goto cleanup;

Does this handle CONFIG_PREEMPT_RT correctly?

The callback_map_uid_match program calls bpf_timer_init(), which the
verifier rejects unconditionally on PREEMPT_RT kernels:

kernel/bpf/verifier.c:process_timer_func() {
    if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
        verbose(env, "bpf_timer cannot be used for PREEMPT_RT.\n");
        return -EOPNOTSUPP;
    }

On a PREEMPT_RT build timer_mim_reject__load() returns -EOPNOTSUPP and
ASSERT_OK(err, "callback_accept_skel_load") reports a test failure.

The existing skip hatch is a few lines later:

    timer_skel = timer_mim__open_and_load();
    if (!timer_skel && errno == EOPNOTSUPP) {
            test__skip();
            return;
    }

Before this patch all timer_mim_reject loads were expected to fail, so
-EOPNOTSUPP satisfied the rejection assertion and control reached the
skip. After this patch the must-succeed load fires first, converting
timer_mim from SKIP to FAIL on PREEMPT_RT.

Should the capability check move ahead of the new positive control, or
should callback_accept_skel_load check for err == -EOPNOTSUPP and call
test__skip()?

Two related issues also follow from this ordering:

- libbpf_set_print(NULL) is still active when the positive control
  fails, so the verifier log is suppressed and only "unexpected error:
  -95" is shown.

- The two ASSERT_ERR() arms for test1 and callback_map_uid_mismatch
  only check err < 0, so on PREEMPT_RT they also pass for the wrong
  reason (-EOPNOTSUPP instead of the intended map_uid mismatch), making
  those negative cases silent tautologies.

> +	timer_mim_reject__destroy(timer_reject_skel);
> +	timer_reject_skel = NULL;
> +	libbpf_set_print(old_print_fn);
>
>  	timer_skel = timer_mim__open_and_load();
>  	if (!timer_skel && errno == EOPNOTSUPP) {

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/timer_mim_reject.c b/tools/testing/selftests/bpf/progs/timer_mim_reject.c
> index dd3f1ed6d6e62..83f31138336b7 100644
> --- a/tools/testing/selftests/bpf/progs/timer_mim_reject.c
> +++ b/tools/testing/selftests/bpf/progs/timer_mim_reject.c
> @@ -43,7 +43,7 @@ static int timer_cb(void *map, int *key, struct hmap_elem *val)
>  	return 0;
>  }
>
> -SEC("fentry/bpf_fentry_test1")
> +SEC("?fentry/bpf_fentry_test1")
>  int BPF_PROG(test1, int a)
>  {

[ ... ]

> +SEC("?fentry/bpf_fentry_test1")
> +int BPF_PROG(callback_map_uid_match, int a)
> +{
> +	struct hmap_elem *val;
> +	struct bpf_map *inner_map;
> +	int array_key = ARRAY_KEY;
> +	int hash_key = HASH_KEY;
> +
> +	inner_map = bpf_map_lookup_elem(&outer_arr, &array_key);
> +	if (!inner_map)
> +		return 0;
> +	val = bpf_map_lookup_elem(inner_map, &hash_key);
> +	if (!val)
> +		return 0;
> +
> +	bpf_timer_init(&val->timer, inner_map, CLOCK_MONOTONIC);
> +	bpf_timer_set_callback(&val->timer, timer_match_cb);
> +	return 0;
> +}


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

Re: [PATCH bpf v1 3/8] bpf: Cancel special fields when recycling rhtab elements

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 527cc5716ee8c..cc60e99ffbe92 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c

[ ... ]

> @@ -2967,8 +2957,8 @@ static int rhtab_delete_elem(struct bpf_rhtab *rhtab, struct rhtab_elem *elem, v
>  		rhtab_read_elem_value(&rhtab->map, copy, elem, flags);
>  		check_and_init_map_value(&rhtab->map, copy);
>  	}
> -	/* Release internal structs: kptr, bpf_timer, task_work, wq */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>  	return 0;
>  }

[ ... ]

> @@ -3009,7 +2999,6 @@ static int rhtab_map_lookup_and_delete_elem(struct bpf_map *map, void *key, void
>  static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *elem, void *value,
>  				      u64 map_flags)
>  {
> -	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>  	void *old_val = rhtab_elem_value(elem, map->key_size);
>
>  	if (map_flags & BPF_NOEXIST)
> @@ -3029,7 +3018,7 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * kptrs/etc. still sit in the slot. Cancel them after the copy
>  	 * to match arraymap's update semantics.
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	bpf_obj_cancel_fields(map, old_val);
>  	return 0;
>  }

A subsystem pattern flags this as potentially concerning: the BPF field
semantics guide states that map operations on field-capable map types that
copy values with copy_map_value() should use the corresponding
bpf_obj_free_fields() on updates. Should this update path still call
bpf_obj_free_fields() to handle kptr references?

rhtab_map_update_existing() overwrites a live map value in place with
copy_map_value_locked() at kernel/bpf/hashtab.c:3008-3010 and then calls
bpf_obj_cancel_fields(). Looking at kernel/bpf/helpers.c:5033,
bpf_obj_cancel_fields() resolves to bpf_map_free_internal_structs() which
only handles BPF_TIMER, BPF_WORKQUEUE and BPF_TASK_WORK:

void bpf_map_free_internal_structs(struct bpf_map *map, void *val)
{
        if (btf_record_has_field(map->record, BPF_TIMER))
                bpf_obj_free_timer(map->record, val);
        if (btf_record_has_field(map->record, BPF_WORKQUEUE))
                bpf_obj_free_workqueue(map->record, val);
        if (btf_record_has_field(map->record, BPF_TASK_WORK))
                bpf_obj_free_task_work(map->record, val);
}

BPF_MAP_TYPE_RHASH appears in the map_check_btf() allowlists for
BPF_KPTR_UNREF, BPF_KPTR_REF, BPF_KPTR_PERCPU and BPF_REFCOUNT in
kernel/bpf/syscall.c:1322-1338. After this change a BPF_EXIST or BPF_ANY
update of an existing rhtab element no longer immediately drops the
referenced kptr held by the value being replaced, and no longer zeroes an
unref kptr - the previous value's kptr stays visible through the new value
until the allocation is reclaimed by rhtab_mem_dtor().

Is this semantic change intentional? Looking at the current tree,
array_map_update_elem() also calls bpf_obj_cancel_fields() in
kernel/bpf/arraymap.c:419 and :427, and the non-prealloc htab free path uses
check_and_cancel_fields() via kernel/bpf/hashtab.c:843, so the
bpf_obj_free_fields()-on-update pattern in the guide may be stale relative
to these implementations. The allocator itself appears safe: bpf_ma_set_dtor()
installs rhtab_mem_dtor() as the bpf_mem_alloc destructor, and every
allocator exit path funnels through free_all() at
kernel/bpf/memalloc.c:265-277 which invokes the destructor, so kptr
references should be released when the allocation is finally destroyed. Is
the intent that rhtab now matches arraymap's deferred-release behaviour, and
if so, should the subsystem documentation be updated to reflect this?

Does the comment at kernel/bpf/hashtab.c:3017-3019 still accurately describe
what the code does? It states:

	/*
	 * copy_map_value() skips special-field offsets, so old timers/
	 * kptrs/etc. still sit in the slot. Cancel them after the copy
	 * to match arraymap's update semantics.
	 */

The comment says old "timers/kptrs/etc." are cancelled after the copy, but
bpf_obj_cancel_fields() only cancels BPF_TIMER, BPF_WORKQUEUE and
BPF_TASK_WORK. Referenced kptrs are deliberately left in the slot - that is
the whole point of this commit ("allowing rhtab_mem_dtor() to release
referenced kptrs when the allocation is eventually destroyed") and is
asserted by the follow-up selftest e5013fee53d7. A reader of this function
is told the opposite of what the code does. Should the comment be updated to
say that only the asynchronous fields are cancelled here and that referenced
kptrs stay attached to the allocation until rhtab_mem_dtor() runs?


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

Re: [PATCH bpf v1 7/8] bpf: Preserve inner map identity in callback frames

[bot+bpf-ci]

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

>     bpf: Preserve inner map identity in callback frames
>
>     Callback frame constructors initialize map-typed argument registers with
>     __mark_reg_known_zero() and then restore map_ptr. This clears map_uid,
>     which is the only field distinguishing inner maps that share an
>     inner_map_meta template.
>
>     When a timer callback invokes bpf_for_each_map_elem() on a second inner
>     map, both the saved first map and the second map value can reach the nested
>     callback as the same template with map_uid zero. bpf_timer_init() then
>     accepts pairing the timer from the second map with the first map.
>
>     The runtime records the first map in the timer without taking a reference.
>     Freeing that map does not find the timer stored in the second map, so a
>     later timer callback dereferences the freed map.
>
>     Copy map_uid from the same caller register as map_ptr when constructing
>     for-each, timer/workqueue, and task-work callback arguments. The existing
>     identity check can then reject mismatched inner maps while allowing a
>     callback value to be paired with its actual map.
>
>     Fixes: 3e8ce29850f1 ("bpf: Prevent pointer mismatch in bpf_timer_init.")
>     Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
>     Fixes: 5c8fd7e2b5b0 ("bpf: bpf task work plumbing")

The Fixes: 69c087ba6225 tag references a commit that cannot contain this
bug. The bug being fixed is that callback frames lose reg->map_uid, but
map_uid did not exist anywhere in the tree at 69c087ba6225 (Feb 2021).

Running 'git grep map_uid 69c087ba6225' returns nothing. BPF timers, which
were the first users of the map/field pairing check, only arrived with
b00628b1c7d5 ("bpf: Introduce bpf timers.", Jul 2021).

The inner-map identity confusion was introduced by 3e8ce29850f1
("bpf: Prevent pointer mismatch in bpf_timer_init.", Jul 2021), which
added map_uid and the (map_ptr, map_uid) identity check now living in
process_map_ptr_arg(). That commit did not update
map_set_for_each_callback_args() to propagate map_uid into the callback
frame.

Should the 69c087ba6225 tag be dropped? The tag only misdirects backporting
(stable AUTOSEL and distro tooling will try to apply the hunk to trees that
predate map_uid, where it does not compile) and misattributes the
regression. The existing 3e8ce29850f1 tag already covers the timer and
for_each hunks, and 5c8fd7e2b5b0 covers the task_work hunk.

---

The Fixes: 3e8ce29850f1 tag is referenced in the commit message, but the
bug in set_timer_callback_state() was actually introduced by an earlier
commit. Commit b00628b1c7d5 ("bpf: Introduce bpf timers.") introduced
set_timer_callback_state() with the pattern of calling
__mark_reg_known_zero() without restoring map_uid.

Commit 3e8ce29850f1 introduced the map_uid field and validation logic, but
set_timer_callback_state() already existed with this buggy pattern at that
point.

Should the Fixes: tag for set_timer_callback_state() point to
b00628b1c7d5 instead of 3e8ce29850f1?


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

Re: [PATCH bpf v1 2/8] selftests/bpf: Test timer field on recycled rhtab element

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
> new file mode 100644
> index 0000000000000..3aad9fc02e063
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define _GNU_SOURCE
> +#include <sched.h>
> +
> +#include <test_progs.h>
> +#include "rhash_timer.skel.h"

A subsystem pattern flags this as potentially concerning: two brand-new
standalone selftest files are added for one narrow internal rhtab code
path - preservation of the BPF_TIMER special field when a deleted rhtab
element's bpf_mem_alloc unit is recycled by an insert.

tools/testing/selftests/bpf/prog_tests/rhash.c (183 lines, subtests
rhash_run, rhash_map_extra_presize, rhash_map_extra_too_big,
rhash_iter_test) and tools/testing/selftests/bpf/progs/rhash.c (248
lines, 7 SEC("syscall") programs) already exist in the same two
directories and cover BPF_MAP_TYPE_RHASH behaviour generally, including
map creation, update/lookup/delete and iteration.

Same-series commit e5013fee53d73 adds rhtab special-field (kptr)
coverage as new cases inside tools/testing/selftests/bpf/prog_tests/
rhash.c and tools/testing/selftests/bpf/progs/rhash.c rather than as a
new file, which is direct evidence of the convention this change departs
from.

Would adding a fifth subtest to test_rhash() plus one more SEC("syscall")
pair in progs/rhash.c give the same coverage without a new test binary
entry, a second skeleton and a duplicated map definition?

[ ... ]

> +	for (attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
> +		err = run_prog(arm_fd, &opts);
> +		if (err) {
> +			ASSERT_OK(err, "arm_deleted_timer");
> +			goto out;
> +		}
> +		if (skel->bss->armed != attempt + 1) {
> +			ASSERT_EQ(skel->bss->armed, attempt + 1, "armed");
> +			goto out;
> +		}
> +		if (skel->bss->timer_init_err) {
> +			ASSERT_OK(skel->bss->timer_init_err, "timer_init_err");
> +			goto out;
> +		}
> +		if (skel->bss->timer_set_callback_err) {
> +			ASSERT_OK(skel->bss->timer_set_callback_err,
> +				  "timer_set_callback_err");
> +			goto out;
> +		}
> +		if (skel->bss->timer_start_err) {
> +			ASSERT_OK(skel->bss->timer_start_err, "timer_start_err");
> +			goto out;
> +		}

This isn't a bug, but can the three diagnostic checks for timer_init_err,
timer_set_callback_err and timer_start_err ever fire?

The armed counter check precedes them and catches every failure that sets
those variables. In progs/rhash_timer.c:arm_timer_cb() each of the three
error paths records the errno and returns 1 without running
__sync_fetch_and_add(&armed, 1), so any of those failures leaves armed
behind attempt + 1, the ASSERT_EQ(skel->bss->armed, attempt + 1, "armed")
fails, and the function goes to out before reaching the three errno
checks.

Armed can never catch up in a later iteration either, since it increments
at most once per arm run. The net effect is that the three per-helper
errno variables - the whole point of which is a precise failure message -
are unreachable, and a bpf_timer_init() failure reports the uninformative
"armed 3 != 4".

The analogous timer_cancel_err check later in the loop is reachable,
because cancel_timer_cb() records the errno without incrementing
cancelled and the test checks timer_cancel_err before it checks
cancelled.

Would checking the three errno variables before the armed count (as is
already done for timer_cancel_err vs cancelled) make the diagnostic
messages reachable?

[ ... ]

This isn't a bug, but inside the loop the same three-line shape is
repeated nine times:

	if (cond) {
		ASSERT_x(...);
		goto out;
	}

instead of the idiomatic one-liner:

	if (!ASSERT_x(...))
		goto out;

The occurrences are in the loop body where the test checks err,
skel->bss->armed, timer_init_err, timer_set_callback_err, timer_start_err,
kern_sync_rcu, update_timer_map, run_prog for cancel, and timer_cancel_err.

The same function already uses the idiomatic form six times immediately
above (after pin_to_first_cpu, after open_and_load, for the three fd
assignments, and after seed_timer_map), so the file is internally
inconsistent and the loop body is roughly twice as long as it needs to be.

Effectively all of tools/testing/selftests/bpf/prog_tests/ uses
if (!ASSERT_x(...)) goto/return, including prog_tests/rhash.c which this
test sits beside. Could the nine blocks be collapsed to
if (!ASSERT_x(...)) goto out?

[ ... ]

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

Re: [PATCH bpf v1 5/8] bpf: Mark NULL kptr stores precise

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete vulnerability fix: missing precision tracking in `bpf_register_is_null()` checks across multiple helper and kfunc argument validation paths.
--

commit b710eacbe0205aaaab5cfce621a3ee0cf1e3f81f
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Mark NULL kptr stores precise

check_map_kptr_access() permits a scalar store into an untrusted kptr
field only when the register is known to contain zero. Unlike other
verifier checks whose outcome depends on a scalar value, it does not mark
that register precise.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d7dd0befbd10e..ba9753bf529af 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4694,8 +4694,15 @@ static int check_map_kptr_access(struct bpf_verifier_env *env,
>  			return ret;
>  	} else if (class == BPF_STX) {
>  		val_reg = reg_state(env, value_regno);
> -		if (!bpf_register_is_null(val_reg) &&
> -		    map_kptr_match_type(env, kptr_field, val_reg, value_regno))
> +		if (bpf_register_is_null(val_reg)) {
> +			/*
> +			 * This store is valid only because the scalar is known to be
> +			 * zero. Mark it precise so another scalar cannot be pruned
> +			 * against this state.
> +			 */
> +			return mark_chain_precision(env, value_regno);
> +		}

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under 
review, but while this commit successfully fixes the precision-tracking 
bug in check_map_kptr_access(), does the exact same vulnerability still 
exist in several other locations where bpf_register_is_null() is used 
without calling mark_chain_precision()?

For example, in check_func_arg():

	if (bpf_register_is_null(reg) && type_may_be_null(arg_type))
		/* A NULL register has a SCALAR_VALUE type, so skip
		 * type checking.
		 */
		goto skip_type_check;

And also later in check_func_arg() for map values:

	case ARG_PTR_TO_MAP_VALUE:
		if (type_may_be_null(arg_type) && bpf_register_is_null(reg))
			return 0;

Similarly, in btf_check_func_arg_match():

			if (bpf_register_is_null(reg) && type_may_be_null(arg->arg_type))
				continue;

And in release_reg():

	if (bpf_register_is_null(reg))
		return 0;

And in check_kfunc_args() for nullable args:

		if (bpf_register_is_null(reg) && type_may_be_null(kf_arg_type))
			continue;

And in check_kfunc_args() for memory buffer lengths:

			if (bpf_register_is_null(buff_reg))
				break;

And in check_mem_reg():

	if (bpf_register_is_null(reg))
		return 0;

And in check_helper_call() for get_local_storage():

		if (!bpf_register_is_null(&regs[BPF_REG_2])) {
			verbose(env, "get_local_storage() doesn't support non-zero flags\n");
			return -EINVAL;
		}
		break;

Since mark_chain_precision() is omitted in these bypass paths, the scalar 0 
remains imprecise. Could an attacker pass an attacker-controlled arbitrary 
scalar on a second pruned path, which would then be incorrectly accepted by 
the verifier when substituted at runtime?

> +		if (map_kptr_match_type(env, kptr_field, val_reg, value_regno))
>  			return -EACCES;
>  	} else if (class == BPF_ST) {
>  		if (insn->imm) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904104203.345917-1-memxor@gmail.com?part=5

Re: [PATCH bpf v1 5/8] bpf: Mark NULL kptr stores precise

[Eduard Zingerman]

On Fri, 2026-09-04 at 12:41 +0200, Kumar Kartikeya Dwivedi wrote:
> check_map_kptr_access() permits a scalar store into an untrusted kptr
> field only when the register is known to contain zero. Unlike other
> verifier checks whose outcome depends on a scalar value, it does not mark
> that register precise.
> 
> A state checkpoint reached with an imprecise zero can therefore prune a
> second path that reaches the store with an arbitrary nonzero scalar. The
> program can write attacker-controlled bits into the kptr field and load
> them back as a PTR_TO_BTF_ID.
> 
> Call mark_chain_precision() before accepting a known-zero register. This
> forces state equivalence to compare its scalar range and makes the verifier
> visit and reject a path carrying a nonzero value.
> 
> Fixes: 61df10c7799e ("bpf: Allow storing unreferenced kptr in map")
> Reported-by: Nicholas Carlini <npc@anthropic.com>
> Suggested-by: Nicholas Carlini <npc@anthropic.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

...

Re: [PATCH bpf v1 0/8] Misc bug fixes - part 4

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Fri,  4 Sep 2026 12:41:51 +0200 you wrote:
> A set of miscellaneous fixes for bugs reported by Nicholas, and GPT-5.6
> when analyzing those fixes, batched together again. See commit logs for
> details. Related rhtab fixes from Yuan Chen and Nuoqi Gui have been
> folded into the series.
> 
> Kumar Kartikeya Dwivedi (5):
>   selftests/bpf: Test timer field on recycled rhtab element
>   bpf: Mark NULL kptr stores precise
>   selftests/bpf: Test imprecise scalar kptr stores
>   bpf: Preserve inner map identity in callback frames
>   selftests/bpf: Test inner map identities in callbacks
> 
> [...]

Here is the summary with links:
  - [bpf,v1,1/8] bpf: Preserve special fields in recycled rhtab elements
    https://git.kernel.org/bpf/bpf/c/5df46ddcb7b3
  - [bpf,v1,2/8] selftests/bpf: Test timer field on recycled rhtab element
    https://git.kernel.org/bpf/bpf/c/dbf6806dc815
  - [bpf,v1,3/8] bpf: Cancel special fields when recycling rhtab elements
    https://git.kernel.org/bpf/bpf/c/65cc95eba9e8
  - [bpf,v1,4/8] selftests/bpf: Test rhtab kptr cancellation semantics
    https://git.kernel.org/bpf/bpf/c/2b97956af608
  - [bpf,v1,5/8] bpf: Mark NULL kptr stores precise
    https://git.kernel.org/bpf/bpf/c/ecdc5043794c
  - [bpf,v1,6/8] selftests/bpf: Test imprecise scalar kptr stores
    https://git.kernel.org/bpf/bpf/c/9dcddf30ac1a
  - [bpf,v1,7/8] bpf: Preserve inner map identity in callback frames
    https://git.kernel.org/bpf/bpf/c/b90c5d770dad
  - [bpf,v1,8/8] selftests/bpf: Test inner map identities in callbacks
    https://git.kernel.org/bpf/bpf/c/e615b9fd4d9d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html