From: Daniel Borkmann <daniel@iogearbox.net> To: Andrew Kanner <andrew.kanner@gmail.com>, bjorn@kernel.org, magnus.karlsson@intel.com, maciej.fijalkowski@intel.com, jonathan.lemon@gmail.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, aleksander.lobakin@intel.com, xuanzhuo@linux.alibaba.com, ast@kernel.org, hawk@kernel.org, john.fastabend@gmail.com Cc: linux-kernel-mentees@lists.linuxfoundation.org, netdev@vger.kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+fae676d3cf469331fc89@syzkaller.appspotmail.com Subject: Re: [PATCH net-next v2] net/xdp: fix zero-size allocation warning in xskq_create() Date: Thu, 5 Oct 2023 00:49:23 +0200 [thread overview] Message-ID: <2f5abbf8-8d50-3deb-19cd-9bfd654e1ceb@iogearbox.net> (raw) In-Reply-To: <20231002222939.1519-1-andrew.kanner@gmail.com> On 10/3/23 12:29 AM, Andrew Kanner wrote: > Syzkaller reported the following issue: > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 2807 at mm/vmalloc.c:3247 __vmalloc_node_range (mm/vmalloc.c:3361) > Modules linked in: > CPU: 0 PID: 2807 Comm: repro Not tainted 6.6.0-rc2+ #12 > Hardware name: Generic DT based system > unwind_backtrace from show_stack (arch/arm/kernel/traps.c:258) > show_stack from dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1)) > dump_stack_lvl from __warn (kernel/panic.c:633 kernel/panic.c:680) > __warn from warn_slowpath_fmt (./include/linux/context_tracking.h:153 kernel/panic.c:700) > warn_slowpath_fmt from __vmalloc_node_range (mm/vmalloc.c:3361 (discriminator 3)) > __vmalloc_node_range from vmalloc_user (mm/vmalloc.c:3478) > vmalloc_user from xskq_create (net/xdp/xsk_queue.c:40) > xskq_create from xsk_setsockopt (net/xdp/xsk.c:953 net/xdp/xsk.c:1286) > xsk_setsockopt from __sys_setsockopt (net/socket.c:2308) > __sys_setsockopt from ret_fast_syscall (arch/arm/kernel/entry-common.S:68) > > xskq_get_ring_size() uses struct_size() macro to safely calculate the > size of struct xsk_queue and q->nentries of desc members. But the > syzkaller repro was able to set q->nentries with the value initially > taken from copy_from_sockptr() high enough to return SIZE_MAX by > struct_size(). The next PAGE_ALIGN(size) is such case will overflow > the size_t value and set it to 0. This will trigger WARN_ON_ONCE in > vmalloc_user() -> __vmalloc_node_range(). > > The issue is reproducible on 32-bit arm kernel. > > Reported-and-tested-by: syzbot+fae676d3cf469331fc89@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/000000000000c84b4705fb31741e@google.com/T/ > Link: https://syzkaller.appspot.com/bug?extid=fae676d3cf469331fc89 > Fixes: 9f78bf330a66 ("xsk: support use vaddr as ring") > Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com> I guess also: Reported-by: syzbot+b132693e925cbbd89e26@syzkaller.appspotmail.com Moreover, this fix is needed in bpf/net tree (as opposed to *-next tree), right? > net/xdp/xsk_queue.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c > index f8905400ee07..b03d1bfb6978 100644 > --- a/net/xdp/xsk_queue.c > +++ b/net/xdp/xsk_queue.c > @@ -34,6 +34,9 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue) > q->ring_mask = nentries - 1; > > size = xskq_get_ring_size(q, umem_queue); > + if (unlikely(size == SIZE_MAX)) > + return NULL; Doesn't this leak q here ? > size = PAGE_ALIGN(size); > > q->ring = vmalloc_user(size); >
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Borkmann via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org> To: Andrew Kanner <andrew.kanner@gmail.com>, bjorn@kernel.org, magnus.karlsson@intel.com, maciej.fijalkowski@intel.com, jonathan.lemon@gmail.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, aleksander.lobakin@intel.com, xuanzhuo@linux.alibaba.com, ast@kernel.org, hawk@kernel.org, john.fastabend@gmail.com Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-kernel@vger.kernel.org, syzbot+fae676d3cf469331fc89@syzkaller.appspotmail.com Subject: Re: [PATCH net-next v2] net/xdp: fix zero-size allocation warning in xskq_create() Date: Thu, 5 Oct 2023 00:49:23 +0200 [thread overview] Message-ID: <2f5abbf8-8d50-3deb-19cd-9bfd654e1ceb@iogearbox.net> (raw) In-Reply-To: <20231002222939.1519-1-andrew.kanner@gmail.com> On 10/3/23 12:29 AM, Andrew Kanner wrote: > Syzkaller reported the following issue: > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 2807 at mm/vmalloc.c:3247 __vmalloc_node_range (mm/vmalloc.c:3361) > Modules linked in: > CPU: 0 PID: 2807 Comm: repro Not tainted 6.6.0-rc2+ #12 > Hardware name: Generic DT based system > unwind_backtrace from show_stack (arch/arm/kernel/traps.c:258) > show_stack from dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1)) > dump_stack_lvl from __warn (kernel/panic.c:633 kernel/panic.c:680) > __warn from warn_slowpath_fmt (./include/linux/context_tracking.h:153 kernel/panic.c:700) > warn_slowpath_fmt from __vmalloc_node_range (mm/vmalloc.c:3361 (discriminator 3)) > __vmalloc_node_range from vmalloc_user (mm/vmalloc.c:3478) > vmalloc_user from xskq_create (net/xdp/xsk_queue.c:40) > xskq_create from xsk_setsockopt (net/xdp/xsk.c:953 net/xdp/xsk.c:1286) > xsk_setsockopt from __sys_setsockopt (net/socket.c:2308) > __sys_setsockopt from ret_fast_syscall (arch/arm/kernel/entry-common.S:68) > > xskq_get_ring_size() uses struct_size() macro to safely calculate the > size of struct xsk_queue and q->nentries of desc members. But the > syzkaller repro was able to set q->nentries with the value initially > taken from copy_from_sockptr() high enough to return SIZE_MAX by > struct_size(). The next PAGE_ALIGN(size) is such case will overflow > the size_t value and set it to 0. This will trigger WARN_ON_ONCE in > vmalloc_user() -> __vmalloc_node_range(). > > The issue is reproducible on 32-bit arm kernel. > > Reported-and-tested-by: syzbot+fae676d3cf469331fc89@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/000000000000c84b4705fb31741e@google.com/T/ > Link: https://syzkaller.appspot.com/bug?extid=fae676d3cf469331fc89 > Fixes: 9f78bf330a66 ("xsk: support use vaddr as ring") > Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com> I guess also: Reported-by: syzbot+b132693e925cbbd89e26@syzkaller.appspotmail.com Moreover, this fix is needed in bpf/net tree (as opposed to *-next tree), right? > net/xdp/xsk_queue.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/xdp/xsk_queue.c b/net/xdp/xsk_queue.c > index f8905400ee07..b03d1bfb6978 100644 > --- a/net/xdp/xsk_queue.c > +++ b/net/xdp/xsk_queue.c > @@ -34,6 +34,9 @@ struct xsk_queue *xskq_create(u32 nentries, bool umem_queue) > q->ring_mask = nentries - 1; > > size = xskq_get_ring_size(q, umem_queue); > + if (unlikely(size == SIZE_MAX)) > + return NULL; Doesn't this leak q here ? > size = PAGE_ALIGN(size); > > q->ring = vmalloc_user(size); > _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2023-10-04 22:50 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-10-02 22:29 [PATCH net-next v2] net/xdp: fix zero-size allocation warning in xskq_create() Andrew Kanner 2023-10-02 22:29 ` Andrew Kanner 2023-10-03 10:26 ` Alexander Lobakin 2023-10-03 10:26 ` Alexander Lobakin 2023-10-04 22:49 ` Daniel Borkmann [this message] 2023-10-04 22:49 ` Daniel Borkmann via Linux-kernel-mentees 2023-10-05 7:35 ` Andrew Kanner 2023-10-05 7:35 ` Andrew Kanner
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=2f5abbf8-8d50-3deb-19cd-9bfd654e1ceb@iogearbox.net \ --to=daniel@iogearbox.net \ --cc=aleksander.lobakin@intel.com \ --cc=andrew.kanner@gmail.com \ --cc=ast@kernel.org \ --cc=bjorn@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=davem@davemloft.net \ --cc=edumazet@google.com \ --cc=hawk@kernel.org \ --cc=john.fastabend@gmail.com \ --cc=jonathan.lemon@gmail.com \ --cc=kuba@kernel.org \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maciej.fijalkowski@intel.com \ --cc=magnus.karlsson@intel.com \ --cc=netdev@vger.kernel.org \ --cc=pabeni@redhat.com \ --cc=syzbot+fae676d3cf469331fc89@syzkaller.appspotmail.com \ --cc=xuanzhuo@linux.alibaba.com \ /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: linkBe 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.