[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;
>  }
>  
> 

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

[Kumar Kartikeya Dwivedi]

On Sun Jul 26, 2026 at 5:51 PM CEST, 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>
> ---

I don't think this is enough. Upon reading the code, I am suspicious about the
check_and_init_map_value() in rhtab_map_update_elem() (post this change). It
will likely end up zeroing non-zero kptrs and leaking them, since recycled
elements will retain the value. Dtor path is fine but that only covers the case
where freed element is never reused and only freed on map destruction.

It might amount to simply remove that function call from the function. The other
user in delete path looks fine since it operates on output buffer.

Please make sure to also add tests for such behavior, having a map of at most
one element should allow for its reuse, you might have to reinvoke the program,
once doing update, then another doing delete, then another doing update to be
able to rellocate the same element. I am sure it can be figured out (with AI's
help), but please validate it using tests.

pw-bot: cr

>  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