[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

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