From: Masoud Aghasi <maghasi@disroot.org>
To: bpf@vger.kernel.org
Cc: andrii@kernel.org, eddyz87@gmail.com, ast@kernel.org,
daniel@iogearbox.net, memxor@gmail.com, martin.lau@linux.dev,
song@kernel.org, yonghong.song@linux.dev, jolsa@kernel.org,
emil@etsalapatis.com, ihor.solodrai@linux.dev,
john.fastabend@gmail.com, brianvv@google.com
Subject: Re: [PATCH] bpf: Fix u32 overflow issue in map batch operations
Date: Mon, 24 Aug 2026 20:15:09 +0100 [thread overview]
Message-ID: <bdfd2ebf-b249-43cd-a393-0088a7accfe0@disroot.org> (raw)
In-Reply-To: <20260824113123.270057-1-maghasi@disroot.org>
On 24/08/2026 12:42, sashiko-bot@kernel.org wrote:
> 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 <maghasi@disroot.org>
>
> 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;
>
As far as I understand it, the key_size + value_size must be less than
4MB at-least on x86_64 systems. so if the bucket_cnt be higher than
1024, an overflow would be possible. Also the possibility of the
bucket_cnt be higher than 1024 depends on the hashing algorithm used.
As the hash is a u32 value and the key of the hash map could be a bigger
type, I would say there is a possibility of overflow here as well, and
we must add the fix for this case too, in the v2.
I will try to look deeper into the hashing algorithm and see whether I
can reproduce a bucket_cnt large enough to trigger the overflow. If
anyone has a more concrete understanding of the bounds here, please let
me know.
prev parent reply other threads:[~2026-08-24 19:15 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-24 11:31 [PATCH] bpf: Fix u32 overflow issue in map batch operations Masoud Aghasi
2026-08-24 11:42 ` sashiko-bot
2026-08-25 10:21 ` Masoud Aghasi
2026-09-02 20:44 ` [PATCH v2] " Masoud Aghasi
2026-09-02 21:50 ` bot+bpf-ci
2026-09-03 8:27 ` [PATCH v3] " Masoud Aghasi
2026-09-03 8:45 ` sashiko-bot
2026-09-03 12:31 ` Masoud Aghasi
2026-09-04 4:45 ` Alexei Starovoitov
2026-09-04 13:19 ` Masoud Aghasi
2026-09-06 22:42 ` Masoud Aghasi
2026-09-06 23:34 ` Masoud Aghasi
2026-08-24 12:17 ` [PATCH] " bot+bpf-ci
2026-08-24 19:15 ` Masoud Aghasi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bdfd2ebf-b249-43cd-a393-0088a7accfe0@disroot.org \
--to=maghasi@disroot.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brianvv@google.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=ihor.solodrai@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.