Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing `size_t` cast in `key_size * bucket_cnt` and `value_size * bucket_cnt` length calculations leads to 32-bit integer overflow and silent data truncation in `__htab_map_lookup_and_delete_batch`.
--
--- Patch [1]: [PATCH] bpf: Fix u32 overflow issue in map batch operations ---
commit c9edb3a32ad168bfaf9df954c22a4563c73796f6
Author: Masoud Aghasi <[email protected]>
bpf: Fix u32 overflow issue in map batch operations
This patch addresses a u32 overflow issue in map batch operations where
address calculations exceeding 4GB could lead to data corruption. It
resolves this by casting the counter variables to size_t before
multiplying them by the map's key or value size.
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index d8db1cebc1936..334eeb297892e 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -1978,9 +1978,9 @@ __htab_map_lookup_and_delete_batch(struct bpf_map *map,
>
> rcu_read_unlock();
> bpf_enable_instrumentation();
> - if (bucket_cnt && (copy_to_user(ukeys + total * key_size, keys,
> + if (bucket_cnt && (copy_to_user(ukeys + (size_t)total * key_size, keys,
> key_size * bucket_cnt) ||
> - copy_to_user(uvalues + total * value_size, values,
> + copy_to_user(uvalues + (size_t)total * value_size, values,
[Severity: High]
This is a pre-existing issue, but does key_size * bucket_cnt or
value_size * bucket_cnt also need a size_t cast here?
If value_size * bucket_cnt exceeds 4GB, the 32-bit integer overflow would
truncate the length argument to copy_to_user() in
__htab_map_lookup_and_delete_batch(), causing it to copy less data than
expected. This could result in silent data loss since the elements are
permanently deleted from the kernel map, while the syscall falsely returns
success.
> value_size * bucket_cnt))) {
> ret = -EFAULT;
> goto after_loop;
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.