All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Song Liu" <song@kernel.org>,
	"Yonghong Song" <yonghong.song@linux.dev>,
	"KP Singh" <kpsingh@kernel.org>,
	"Stanislav Fomichev" <sdf@google.com>,
	"Hao Luo" <haoluo@google.com>, "Jiri Olsa" <jolsa@kernel.org>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: syzbot+8cd36f6b65f3cafd400a@syzkaller.appspotmail.com,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: [PATCH bpf v2 1/2] bpf: Fix DEVMAP_HASH overflow check on 32-bit arches
Date: Thu, 29 Feb 2024 12:22:47 +0100	[thread overview]
Message-ID: <20240229112250.13723-2-toke@redhat.com> (raw)
In-Reply-To: <20240229112250.13723-1-toke@redhat.com>

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@google.com
Reported-and-tested-by: syzbot+8cd36f6b65f3cafd400a@syzkaller.appspotmail.com
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 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..d08888e5f994 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 + 1)
 			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


  reply	other threads:[~2024-02-29 11:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29 11:22 [PATCH bpf v2 0/2] Fix hashmap overflow checks for 32-bit arches Toke Høiland-Jørgensen
2024-02-29 11:22 ` Toke Høiland-Jørgensen [this message]
2024-02-29 11:22 ` [PATCH bpf v2 2/2] bpf: Fix hashtab overflow check on " Toke Høiland-Jørgensen
2024-02-29 17:07   ` Alexei Starovoitov
2024-02-29 22:21     ` John Fastabend
2024-03-01 12:35       ` Toke Høiland-Jørgensen
2024-03-01 17:15         ` Alexei Starovoitov
2024-03-04 13:02           ` Toke Høiland-Jørgensen
2024-03-06  5:29             ` Alexei Starovoitov
2024-03-06 10:32               ` Toke Høiland-Jørgensen
2024-03-06 16:53                 ` Alexei Starovoitov
2024-03-07 12:00                   ` Toke Høiland-Jørgensen
2024-03-01 17:21         ` John Fastabend

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=20240229112250.13723-2-toke@redhat.com \
    --to=toke@redhat.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=syzbot+8cd36f6b65f3cafd400a@syzkaller.appspotmail.com \
    --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.