The devmap code allocates a number hash buckets equal to the next power of two
of the max_entries value provided when creating the map. When rounding up to the
next power of two, the 32-bit variable storing the number of buckets can
overflow, and the code checks for overflow by checking if the truncated 32-bit value
is equal to 0. However, on 32-bit arches the rounding up itself can overflow
mid-way through, because it ends up doing a left-shift of 32 bits on an unsigned
long value. If the size of an unsigned long is four bytes, this is undefined
behaviour, so there is no guarantee that we'll end up with a nice and tidy
0-value at the end.
Syzbot managed to turn this into a crash on arm32 by creating a DEVMAP_HASH with
max_entries > 0x80000000 and then trying to update it. Fix this by moving the
overflow check to before the rounding up operation.
Fixes: 6f9d451ab1a3 ("xdp: Add devmap_hash map type for looking up devices by hashed index")
Link: https://lore.kernel.org/r/000000000000ed666a0611af6818@xxxxxxxxxx
Reported-and-tested-by: syzbot+8cd36f6b65f3cafd400a@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
---
kernel/bpf/devmap.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index a936c704d4e7..9b2286f9c6da 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -130,13 +130,11 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr)
bpf_map_init_from_attr(&dtab->map, attr);
if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
- dtab->n_buckets = roundup_pow_of_two(dtab->map.max_entries);
-
- if (!dtab->n_buckets) /* Overflow check */
+ if (dtab->map.max_entries > U32_MAX / 2)
return -EINVAL;
- }
- if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
+ dtab->n_buckets = roundup_pow_of_two(dtab->map.max_entries);
+
dtab->dev_index_head = dev_map_create_hash(dtab->n_buckets,
dtab->map.numa_node);
if (!dtab->dev_index_head)
--
2.43.2
