[PATCH bpf-next 0/2] bpf: Fix RHash special-field recycling

[PATCH bpf-next 0/2] bpf: Fix RHash special-field recycling

[Nuoqi Gui]

Commit 6905f8601298 ("bpf: Allow special fields in resizable hashtab") says
that "kptr semantics under in-place updates are identical to array map."
However, RHash calls bpf_obj_free_fields() after copy_map_value(), dropping
the kptr that the copy preserves.

Use bpf_obj_cancel_fields() in the update and deferred deletion paths, as
hash and array maps do. It cancels timer, workqueue, and task-work state
while the allocator destructor releases kptrs at final reclamation.

Add a selftest that verifies a task kptr survives a BPF_EXIST update and
exercises delayed deletion.

Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
Nuoqi Gui (2):
      bpf: Cancel RHash special fields on value recycle
      selftests/bpf: Cover RHash special-field recycle

 kernel/bpf/hashtab.c                           | 14 ++---
 tools/testing/selftests/bpf/prog_tests/rhash.c |  3 +
 tools/testing/selftests/bpf/progs/rhash.c      | 81 ++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 7 deletions(-)
---
base-commit: a23a71823352e2d792dcaae25f1ebb744acbfc0b
change-id: 20260722-f01-23-rhash-cancel-bpf-next-02a6e7ec115e

Best regards,
--  
Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

[PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle

[Nuoqi Gui]

Commit 6905f8601298 ("bpf: Allow special fields in resizable hashtab") says
that "kptr semantics under in-place updates are identical to array map."
However, after copy_map_value() preserves special fields,
rhtab_map_update_existing() calls bpf_obj_free_fields() and drops retained
kptrs. BPF_EXIST can therefore unexpectedly clear a kptr.

Use bpf_obj_cancel_fields() in the update and deferred deletion paths, as
hash and array maps do. It cancels timer, workqueue, and task-work state
while the allocator destructor releases kptrs at final reclamation.

Fixes: 6905f8601298 ("bpf: Allow special fields in resizable hashtab")
Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
 kernel/bpf/hashtab.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 9f394e1aa2e8..54ea111daa8b 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -2865,14 +2865,14 @@ 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)
+static void rhtab_check_and_cancel_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));
+	bpf_obj_cancel_fields(&rhtab->map,
+			      rhtab_elem_value(elem, rhtab->map.key_size));
 }
 
 static void rhtab_mem_dtor(void *obj, void *ctx)
@@ -2964,8 +2964,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);
+	/* Cancel reusable internal structs: bpf_timer, task_work, wq */
+	rhtab_check_and_cancel_fields(rhtab, elem);
 	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
 	return 0;
 }
@@ -3027,7 +3027,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);
+	rhtab_check_and_cancel_fields(rhtab, elem);
 	return 0;
 }
 

-- 
2.34.1

Re: [PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle

[sashiko-bot]

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

New issues:
- [High] The `rhtab_map_update_existing` function fails to call `bpf_obj_free_fields()` when overwriting an existing map value, explicitly violating BTF special field cleanup rules.

Pre-existing issues:
- [Critical] Uninitialized spinlock acquired in `alloc_htab_elem` when `BPF_F_LOCK` is used.
- [Critical] BTF special field (kptr) leak and dangerous reuse in preallocated hash maps.
- [High] Missing field cancellation on locked update in `htab_map_update_elem`.
--

--- Patch [1]: [PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle ---
commit b285c707d0a30a431d69802916cd425986e713ab
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

bpf: Cancel RHash special fields on value recycle

The patch replaces bpf_obj_free_fields() with bpf_obj_cancel_fields() in
the update and deferred deletion paths for rhtab. It cancels timers and
workqueues while deferring kptr release.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 9f394e1aa2e85..54ea111daa8bd 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -3027,7 +3027,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);
> +	rhtab_check_and_cancel_fields(rhtab, elem);
>  	return 0;
>  }

[Severity: High]
Does replacing bpf_obj_free_fields() with bpf_obj_cancel_fields() violate
the subsystem guidelines for map updates?

According to the BPF documentation, when overwriting an existing map value
with userspace data using copy_map_value(), bpf_obj_free_fields() must be
called to release resources held by the old value. 

Does using bpf_obj_cancel_fields() here instead fail to release resources
like kptrs during the update?


[Severity: High]
This is a pre-existing issue, but are special fields left active during
locked updates in htab_map_update_elem()?

Looking at kernel/bpf/hashtab.c:htab_map_update_elem():

    if (map_flags & BPF_F_LOCK) {
        ...
        copy_map_value_locked(map, htab_elem_value(l_old, key_size), value, false);
        return 0;
    }

When BPF_F_LOCK is used to overwrite an existing element in-place,
copy_map_value_locked() is called and the function returns 0. 

Is the cancellation step skipped here, leaving old special fields like
timers or workqueues running indefinitely?


[Severity: Critical]
This is a pre-existing issue, but could alloc_htab_elem() be acquiring an
uninitialized spinlock?

Looking at kernel/bpf/hashtab.c:alloc_htab_elem():

    l_new = bpf_mem_cache_alloc(&htab->ma);
    ...
    if (map_flags & BPF_F_LOCK)
        copy_map_value_locked(&htab->map, htab_elem_value(l_new, key_size), value, false);

When updating a non-preallocated hash map with BPF_F_LOCK for a new key,
the new element is allocated but not initialized. 

Because check_and_init_map_value() is not called before
copy_map_value_locked(), will the kernel attempt to lock an uninitialized
spinlock in the new memory?


[Severity: Critical]
This is a pre-existing issue, but can preallocated hash maps leak kptrs
and improperly reuse them across elements?

When an element is deleted, kernel/bpf/hashtab.c:free_htab_elem() cancels
timers but doesn't free fields:

    check_and_cancel_fields(htab, l);
    pcpu_freelist_push(&htab->freelist, &l->fnode);

Later, kernel/bpf/hashtab.c:alloc_htab_elem() pops the element and skips
initialization:

    l_new = prealloc_lru_pop(htab, key, hash);
    if (!l_new)
        ...

Because bpf_obj_free_fields() is skipped on deletion and
check_and_init_map_value() is skipped on reallocation, would the new
element inherit the old kptrs? 

Might this cause kptr resources to leak or be improperly accessed by the
reallocated element?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn?part=1

Re: [PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle

[Mykyta Yatsenko]

On 7/26/26 4:51 PM, Nuoqi Gui wrote:
> Commit 6905f8601298 ("bpf: Allow special fields in resizable hashtab") says
> that "kptr semantics under in-place updates are identical to array map."
> However, after copy_map_value() preserves special fields,
> rhtab_map_update_existing() calls bpf_obj_free_fields() and drops retained
> kptrs. BPF_EXIST can therefore unexpectedly clear a kptr.
> 
> Use bpf_obj_cancel_fields() in the update and deferred deletion paths, as
> hash and array maps do. It cancels timer, workqueue, and task-work state
> while the allocator destructor releases kptrs at final reclamation.
> 
> Fixes: 6905f8601298 ("bpf: Allow special fields in resizable hashtab")
> Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
> ---

The change looks correct, corresponding htab fix was landed around
the same time as rhtab, so it was missed, and not replicated.

This patch correctly updates update/deletion callsites, which guarantees
that potentially NMI-unsafe kptr destructor is not called from NMI.
Final destructor rhtab_mem_dtor() left unchanged.

Acked-by: Mykyta Yatsenko <yatsenko@meta.com>

>  kernel/bpf/hashtab.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 9f394e1aa2e8..54ea111daa8b 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -2865,14 +2865,14 @@ 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)
> +static void rhtab_check_and_cancel_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));
> +	bpf_obj_cancel_fields(&rhtab->map,
> +			      rhtab_elem_value(elem, rhtab->map.key_size));
>  }
>  
>  static void rhtab_mem_dtor(void *obj, void *ctx)
> @@ -2964,8 +2964,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);
> +	/* Cancel reusable internal structs: bpf_timer, task_work, wq */
> +	rhtab_check_and_cancel_fields(rhtab, elem);
>  	bpf_mem_cache_free_rcu(&rhtab->ma, elem);
>  	return 0;
>  }
> @@ -3027,7 +3027,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);
> +	rhtab_check_and_cancel_fields(rhtab, elem);
>  	return 0;
>  }
>  
> 

[PATCH bpf-next 2/2] selftests/bpf: Cover RHash special-field recycle

[Nuoqi Gui]

Add regression coverage for RHash value recycling with referenced kptrs.

Store a referenced task kptr and update the value with BPF_EXIST. Verify
that bpf_kptr_xchg() still returns the kptr. Then store another task kptr
and delete the element to exercise the delayed reclamation path.

Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
---
 tools/testing/selftests/bpf/prog_tests/rhash.c |  3 +
 tools/testing/selftests/bpf/progs/rhash.c      | 81 ++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/rhash.c b/tools/testing/selftests/bpf/prog_tests/rhash.c
index 98bb66907b7f..5bf2de828361 100644
--- a/tools/testing/selftests/bpf/prog_tests/rhash.c
+++ b/tools/testing/selftests/bpf/prog_tests/rhash.c
@@ -180,4 +180,7 @@ void test_rhash(void)
 
 	if (test__start_subtest("test_rhash_iter"))
 		rhash_iter_test();
+
+	if (test__start_subtest("test_rhash_special_fields_recycle"))
+		rhash_run("test_rhash_special_fields_recycle");
 }
diff --git a/tools/testing/selftests/bpf/progs/rhash.c b/tools/testing/selftests/bpf/progs/rhash.c
index fc2dac3a719e..d28d115cb74e 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, 4);
+	__type(key, int);
+	__type(value, struct special_elem);
+} special_fields SEC(".maps");
+
+struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+void bpf_task_release(struct task_struct *p) __ksym;
+
 SEC("syscall")
 int test_rhash_lookup_update(void *ctx)
 {
@@ -246,3 +262,68 @@ int test_rhash_delete_nonexistent(void *ctx)
 	err = 0;
 	return 0;
 }
+
+SEC("syscall")
+int test_rhash_special_fields_recycle(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)
+		return 6;
+
+	err = 7;
+	task = bpf_kptr_xchg(&elem->task, NULL);
+	if (!task)
+		return 7;
+	bpf_task_release(task);
+
+	err = 8;
+	task = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!task)
+		return 8;
+
+	err = 9;
+	old = bpf_kptr_xchg(&elem->task, task);
+	if (old) {
+		bpf_task_release(old);
+		return 9;
+	}
+
+	err = 10;
+	if (bpf_map_delete_elem(&special_fields, &key))
+		return 10;
+
+	err = 0;
+	return 0;
+}

-- 
2.34.1

Re: [PATCH bpf-next 2/2] selftests/bpf: Cover RHash special-field recycle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The RHash map implementation leaks BTF special fields (such as kptrs) during in-place updates and element recycling by failing to call bpf_obj_free_fields().
- [Medium] The new selftest test_rhash_special_fields_recycle incorrectly asserts and enforces a kernel memory leak regarding BTF special fields.
--

--- Patch [2]: [PATCH bpf-next 2/2] selftests/bpf: Cover RHash special-field recycle ---
commit d7b4034e22767ea5cd89689a56855953c3b5c45a
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

selftests/bpf: Cover RHash special-field recycle

This commit adds regression coverage for RHash value recycling with
referenced kptrs. It stores a referenced task kptr and updates the value
with BPF_EXIST, then verifies that bpf_kptr_xchg() still returns the kptr.
Finally, it deletes the element to exercise the delayed reclamation path.

> diff --git a/tools/testing/selftests/bpf/progs/rhash.c b/tools/testing/selftests/bpf/progs/rhash.c
> index fc2dac3a719e9..d28d115cb74e7 100644
> --- a/tools/testing/selftests/bpf/progs/rhash.c
> +++ b/tools/testing/selftests/bpf/progs/rhash.c

[ ... ]

> +	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)
> +		return 6;
> +
> +	err = 7;
> +	task = bpf_kptr_xchg(&elem->task, NULL);
> +	if (!task)
> +		return 7;

[Severity: Medium]
Is it intentional for this test to codify a kernel memory leak as a valid
feature?

The test updates the element using bpf_map_update_elem() with val2 (which
has no task kptr) but then expects the old kptr to still be present and valid
in the map.

According to the BPF subsystem guidelines, when a map element is overwritten,
bpf_obj_free_fields() must be called to release resources held by the old
value. By asserting that the old kptr survives the map update, the test
appears to enforce a bug where bpf_obj_free_fields() is missing.

[Severity: High]
Could this highlight a resource leak in the underlying RHash map
implementation?

Looking at the RHash implementation in kernel/bpf/hashtab.c,
rhtab_map_update_existing() appears to overwrite the old value using
copy_map_value() but misses the required bpf_obj_free_fields() call.

Additionally, when deleting an element:

kernel/bpf/hashtab.c:rhtab_delete_elem() {
    ...
    /* Cancel reusable internal structs: bpf_timer, task_work, wq */
    rhtab_check_and_cancel_fields(rhtab, elem);
    ...
}

It seems to skip freeing kptrs, leaking the references. Should
bpf_obj_free_fields() be called during these operations to properly release
kernel resources like task_structs?

> +	bpf_task_release(task);
> +
> +	err = 8;
> +	task = bpf_task_acquire(bpf_get_current_task_btf());
> +	if (!task)
> +		return 8;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn?part=2