lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <eb61966f-8666-80f6-1eab-c89bffe496b8@linux.dev> Date: Fri, 6 Oct 2023 16:58:18 -0700 From: Martin KaFai Lau <martin.lau@...ux.dev> To: Andrew Kanner <andrew.kanner@...il.com> Cc: linux-kernel-mentees@...ts.linuxfoundation.org, netdev@...r.kernel.org, bpf@...r.kernel.org, linux-kernel@...r.kernel.org, syzbot+fae676d3cf469331fc89@...kaller.appspotmail.com, syzbot+b132693e925cbbd89e26@...kaller.appspotmail.com, bjorn@...nel.org, magnus.karlsson@...el.com, maciej.fijalkowski@...el.com, jonathan.lemon@...il.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, aleksander.lobakin@...el.com, xuanzhuo@...ux.alibaba.com, ast@...nel.org, hawk@...nel.org, john.fastabend@...il.com, daniel@...earbox.net Subject: Re: [PATCH bpf v3] net/xdp: fix zero-size allocation warning in xskq_create() On 10/6/23 4:24 PM, Andrew Kanner wrote: >> Thanks for the explanation, so iiuc it means it will overflow the >> struct_size() first because of the is_power_of_2(nentries) requirement? >> Could you help adding some comment to explain? Thanks. >> > The overflow happens because there's no upper limit for nentries > (userspace input). Let me add more context, e.g. from net/xdp/xsk.c: > > static int xsk_setsockopt(struct socket *sock, int level, int optname, > sockptr_t optval, unsigned int optlen) > { > [...] > if (copy_from_sockptr(&entries, optval, sizeof(entries))) > return -EFAULT; > [...] > err = xsk_init_queue(entries, q, false); > [...] > } > > 'entries' is passed to xsk_init_queue() and there're 2 checks: for 0 > and is_power_of_2() only, no upper bound check: > > static int xsk_init_queue(u32 entries, struct xsk_queue **queue, > bool umem_queue) > { > struct xsk_queue *q; > > if (entries == 0 || *queue || !is_power_of_2(entries)) > return -EINVAL; > > q = xskq_create(entries, umem_queue); > if (!q) > return -ENOMEM; > [...] > } > > The 'entries' value is next passed to struct_size() in > net/xdp/xsk_queue.c. If it's large enough - SIZE_MAX will be returned. All make sense. I was mostly asking to add a comment at the "if (unlikely(size == SIZE_MAX)" check to explain this details on why checking SIZE_MAX is enough.
Powered by blists - more mailing lists