Re: [PATCH RFC bpf-next v2 08/18] bpf: Implement iterator APIs for resizable hashtab

> On Aug 14, 2026, at 10:19 PM, Andrii Nakryiko <[email protected]> wrote:
> 
> On Fri, Aug 14, 2026 at 1:15 PM Roman Gushchin <[email protected]> wrote:
>> 
>> 
>>>> On Aug 14, 2026, at 9:49 PM, Andrii Nakryiko <[email protected]> wrote:
>>> 
>>> Is it really a good use of tokens for sashiko to review old patches
>>> that have either landed or were significantly changed since (and
>>> probably got a bunch of new revisions sent), or just plain forgotten
>>> by now?
>>> 
>>> Did something regress recently? I'm not sure this has happened before
>>> until recently...
>> 
>> Ooops, it’s probably a recent regression I had which blocked sashiko reviews for few hours,
>> so I had to restart some patch(set) semi-manually. Likely this one accidentally got into this group.
>> Should be a one time off though.
> 
> It's not just this patch, we've had quite a bunch of sashiko reviews
> for months-old patches in the last few days. Just wanted to raise
> awareness as this doesn't seem to be intentional.
> 
> But ok, let's hope this is a one-time glitch :)

It should not happen anymore, they all should have been sent yesterday, whatever the number was.
If you see any new coming, please, let me know.

Thanks 




> 
>> 
>> Thanks for reporting!
>> 
>> 
>>> 
>>> -- Andrii
>>> 
>>>> On Thu, Aug 13, 2026 at 4:59 PM <[email protected]> wrote:
>>>> 
>>>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>>>> - [High] Uninitialized memory read in `last_key` when `seq_show` overflows.
>>>> - [High] `rhashtable_walk_enter_from` resets iterator on deleted cursor key, causing O(N^2) complexity and infinite loops.
>>>> - [Medium] Missing __GFP_NOWARN flag in user-controlled kmalloc for map->key_size can lead to OOM splats.
>>>> --
>>>> 
>>>> --- Patch [8]: [PATCH RFC bpf-next v2 08/18] bpf: Implement iterator APIs for resizable hashtab ---
>>>> commit 226ce1988038cf4876ff0a72b5ae8e5ebc263151
>>>> Author: Mykyta Yatsenko <[email protected]>
>>>> 
>>>> bpf: Implement iterator APIs for resizable hashtab
>>>> 
>>>> Wire up seq_file BPF iterator for BPF_MAP_TYPE_RHASH so that
>>>> bpf_iter and bpftool map dump work with resizable hash maps.
>>>> 
>>>>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>>>>> index 4611671bf8e74..5ca4d26e70178 100644
>>>>> --- a/kernel/bpf/hashtab.c
>>>>> +++ b/kernel/bpf/hashtab.c
>>>> 
>>>> [ ... ]
>>>> 
>>>>> @@ -3205,36 +3218,113 @@ struct bpf_iter_seq_rhash_map_info {
>>>>>     struct bpf_map *map;
>>>>>     struct bpf_rhtab *rhtab;
>>>>>     struct rhashtable_iter iter;
>>>>> -     u32 skip_elems;
>>>>> +     void *last_key;
>>>>>     bool iter_active;
>>>>> };
>>>>> 
>>>>> static void *bpf_rhash_map_seq_start(struct seq_file *seq, loff_t *pos)
>>>>> {
>>>>> -     return NULL;
>>>>> +     struct bpf_iter_seq_rhash_map_info *info = seq->private;
>>>>> +     struct rhtab_elem *elem;
>>>>> +     void *key = *pos > 0 ? info->last_key : NULL;
>>>>> +
>>>>> +     scoped_guard(rcu) {
>>>>> +             rhashtable_walk_enter_from(&info->rhtab->ht, &info->iter,
>>>>> +                                        key, info->rhtab->params);
>>>> 
>>>> [Severity: High]
>>>> Will this silently restart iteration from bucket 0 if the cursor key is deleted?
>>>> 
>>>> rhashtable_walk_enter_from() looks up the provided cursor key to resume
>>>> iteration. If the key is missing, it returns early and leaves the iterator at
>>>> bucket 0 (iter->slot = 0).
>>>> 
>>>> In __rhtab_map_lookup_and_delete_batch(), the batch operation deletes elements
>>>> and returns the last deleted key to userspace. On the next batch syscall, the
>>>> user passes this deleted key back as the cursor. Since the key no longer
>>>> exists, rhashtable_walk_enter_from() resets the iterator to 0.
>>>> 
>>>> The inner loop then executes while (total < max_count) holding rcu_read_lock(),
>>>> rescanning all previously emptied buckets from the beginning of the table.
>>>> 
>>>> For lookup_batch and seq_file iterators, does this yield massive duplicate
>>>> outputs to userspace and enormous RCU stall times?
>>>> 
>>>>> +             rhashtable_walk_start(&info->iter);
>>>>> +     }
>>>>> +     info->iter_active = true;
>>>>> +
>>>>> +     elem = rhtab_iter_next(&info->iter);
>>>>> +     if (!elem)
>>>>> +             return NULL;
>>>>> +     /*
>>>>> +      * if *pos is not 0, previously iteration failed on this elem,
>>>>> +      * so we are restarting it. That's why no need to increment *pos.
>>>>> +      */
>>>>> +     if (*pos == 0)
>>>>> +             ++*pos;
>>>> 
>>>> [Severity: High]
>>>> Does this pass uninitialized memory as a hash key if seq_show() output
>>>> overflows?
>>>> 
>>>> When reading a BPF iterator, seq_read_iter() calls bpf_rhash_map_seq_start(),
>>>> which increments *pos from 0 to 1 and returns the first element.
>>>> 
>>>> If the output of seq_show() for this element exceeds the initial seq_file
>>>> buffer, the buffer overflows. The sequence machinery reallocates a larger
>>>> buffer and calls seq_start() again.
>>>> 
>>>> Since *pos is now 1, the code executes key = info->last_key. However, last_key
>>>> is only populated in seq_next(), which hasn't been called yet. Thus, last_key
>>>> remains uninitialized from its kmalloc(..., GFP_USER) allocation.
>>>> 
>>>>> +     return elem;
>>>>> }
>>>> 
>>>> [ ... ]
>>>> 
>>>>> static int bpf_iter_init_rhash_map(void *priv_data, struct bpf_iter_aux_info *aux)
>>>>> {
>>>>> +     struct bpf_iter_seq_rhash_map_info *info = priv_data;
>>>>> +     struct bpf_map *map = aux->map;
>>>>> +
>>>>> +     info->last_key = kmalloc(map->key_size, GFP_USER);
>>>> 
>>>> [Severity: Medium]
>>>> Does this need __GFP_NOWARN?
>>>> 
>>>> Since BPF hash map key sizes can be extremely large, requesting large
>>>> contiguous memory blocks without __GFP_NOWARN can fail loudly under memory
>>>> fragmentation and trigger high-order page allocation warnings.
>>>> 
>>>>> +     if (!info->last_key)
>>>>> +             return -ENOMEM;
>>>>> +
>>>>> +     bpf_map_inc_with_uref(map);
>>>>> +     info->map = map;
>>>>> +     info->rhtab = container_of(map, struct bpf_rhtab, map);
>>>>> +     info->iter_active = false;
>>>>>     return 0;
>>>>> }
>>>> 
>>>> --
>>>> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
>>>>