Skip to content

L
linux
Switch branch/tag
  1. 08 Mar, 2024 4 commits
    • Alexei Starovoitov's avatar
      Merge branch 'fix-hash-bucket-overflow-checks-for-32-bit-arches' · a27e8967
      Alexei Starovoitov authored 
      Toke Høiland-Jørgensen says:

====================
Fix hash bucket overflow checks for 32-bit arches

Syzbot managed to trigger a crash by creating a DEVMAP_HASH map with a
large number of buckets because the overflow check relies on
well-defined behaviour that is only correct on 64-bit arches.

Fix the overflow checks to happen before values are rounded up in all
the affected map types.

v3:
- Keep the htab->n_buckets > U32_MAX / sizeof(struct bucket) check
- Use 1UL << 31 instead of U32_MAX / 2 + 1 as the constant to check
  against
- Add patch to fix stackmap.c
v2:
- Fix off-by-one error in overflow check
- Apply the same fix to hashtab, where the devmap_hash code was copied
  from (John)

Toke Høiland-Jørgensen (3):
  bpf: Fix DEVMAP_HASH overflow check on 32-bit arches
  bpf: Fix hashtab overflow check on 32-bit arches
  bpf: Fix stackmap overflow check on 32-bit arches

 kernel/bpf/devmap.c   | 11 ++++++-----
 kernel/bpf/hashtab.c  | 14 +++++++++-----
 kernel/bpf/stackmap.c |  9 ++++++---
 3 files changed, 21 insertions(+), 13 deletions(-)
====================

Link: https://lore.kernel.org/r/20240307120340.99577-1-toke@redhat.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      a27e8967
    • Toke Høiland-Jørgensen's avatar
      bpf: Fix stackmap overflow check on 32-bit arches · 7a4b2125
      Toke Høiland-Jørgensen authored 
      The stackmap code relies on roundup_pow_of_two() to compute the number
of hash buckets, and contains an overflow check by checking if the
resulting value is 0. However, on 32-bit arches, the roundup code itself
can overflow by doing a 32-bit left-shift of an unsigned long value,
which is undefined behaviour, so it is not guaranteed to truncate
neatly. This was triggered by syzbot on the DEVMAP_HASH type, which
contains the same check, copied from the hashtab code.

The commit in the fixes tag actually attempted to fix this, but the fix
did not account for the UB, so the fix only works on CPUs where an
overflow does result in a neat truncation to zero, which is not
guaranteed. Checking the value before rounding does not have this
problem.

Fixes: 6183f4d3 ("bpf: Check for integer overflow when using roundup_pow_of_two()")
Signed-off-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: default avatarBui Quang Minh <minhquangbui99@gmail.com>
Message-ID: <20240307120340.99577-4-toke@redhat.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      7a4b2125
    • Toke Høiland-Jørgensen's avatar
      bpf: Fix hashtab overflow check on 32-bit arches · 6787d916
      Toke Høiland-Jørgensen authored 
      The hashtab code relies on roundup_pow_of_two() to compute the number of
hash buckets, and contains an overflow check by checking if the
resulting value is 0. However, on 32-bit arches, the roundup code itself
can overflow by doing a 32-bit left-shift of an unsigned long value,
which is undefined behaviour, so it is not guaranteed to truncate
neatly. This was triggered by syzbot on the DEVMAP_HASH type, which
contains the same check, copied from the hashtab code. So apply the same
fix to hashtab, by moving the overflow check to before the roundup.

Fixes: daaf427c ("bpf: fix arraymap NULL deref and missing overflow and zero size checks")
Signed-off-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Message-ID: <20240307120340.99577-3-toke@redhat.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      6787d916
    • Toke Høiland-Jørgensen's avatar
      bpf: Fix DEVMAP_HASH overflow check on 32-bit arches · 281d464a
      Toke Høiland-Jørgensen authored 
      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: 6f9d451a ("xdp: Add devmap_hash map type for looking up devices by hashed index")
Link: https://lore.kernel.org/r/000000000000ed666a0611af6818@google.com
Reported-and-tested-by: syzbot+8cd36f6b65f3cafd400a@syzkaller.appspotmail.com
Signed-off-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Message-ID: <20240307120340.99577-2-toke@redhat.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      281d464a
  3. 07 Mar, 2024 7 commits
  5. 06 Mar, 2024 25 commits
  7. 04 Mar, 2024 4 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7