Re: [PATCH] bpf: Fix u32 overflow issue in map batch operations

> Fixes: cb4d03ab499d ("bpf: Add generic support for lookup batch op")

The single Fixes: tag covers only one of the three commits whose code this
patch corrects.

cb4d03ab499d introduced generic_map_lookup_batch() - that tag is correct
for the changes in kernel/bpf/syscall.c at generic_map_lookup_batch().

The other hunks fix different commits:

generic_map_delete_batch() and generic_map_update_batch() were added by
aa2e93b8e58e ("bpf: Add generic support for update and delete batch ops"),
which introduced the buggy expressions:

    +       key = __bpf_copy_key(keys + cp * map->key_size, map->key_size);
    +       if (copy_from_user(value, values + cp * value_size, value_size))

__htab_map_lookup_and_delete_batch() was added by 057996380a42 ("bpf: Add
batch ops to all htab bpf map"), which introduced:

    +   if (bucket_cnt && (copy_to_user(ukeys + total * key_size, keys,
    +       key_size * bucket_cnt) ||
    +       copy_to_user(uvalues + total * value_size, values,

Without these tags the stable backport scope is wrong: a tree that has
aa2e93b8e58e or 057996380a42 but predates cb4d03ab499d (or vice versa)
will not be matched.

Would it make sense to add:

  Fixes: aa2e93b8e58e ("bpf: Add generic support for update and delete batch ops")
  Fixes: 057996380a42 ("bpf: Add batch ops to all htab bpf map")

> This patch resolves the mentioned issues by converting the cp's type
> to size_t in the effected places.

The changelog misdescribes what the patch does, in two ways.

First, no type is converted. The diff only adds (size_t) casts at the use
sites; every declaration is untouched. generic_map_lookup_batch() still
declares "u32 value_size, cp, max_count;" at kernel/bpf/syscall.c,
generic_map_update_batch() still declares "u32 value_size, cp, max_count;",
and generic_map_delete_batch() still declares "u32 cp, max_count;".

This matters beyond wording, because actually converting cp to size_t would
have been a bug: all three functions report progress with

    copy_to_user(&uattr->batch.count, &cp, sizeof(cp))

and uattr->batch.count is __u32 in the uapi, so a size_t cp would write
8 bytes into a 4-byte field on 64-bit. Casting at the use site, as the
patch actually does, is the correct approach - the changelog should
describe it that way so a reviewer does not "simplify" it later.

Second, there is no cp in the hashtab.c hunk. __htab_map_lookup_and_delete_batch()
uses total (declared "u32 bucket_cnt, total, key_size, value_size;" at
kernel/bpf/hashtab.c), so "the cp's type" does not describe that change.

(Also "effected places" should read "affected places".)


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32723998558