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 <[email protected]>
> ---
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 <[email protected]>
> 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;
> }
>
>
