Michael S. Tsirkin
2023-Jan-20 13:09 UTC
[PATCH v1 5/6] virtio_net: Guard against buffer length overflow in xdp_linearize_page()
On Thu, Jan 19, 2023 at 03:57:20PM +0200, Alexander Shishkin wrote:> When reassembling incoming buffers to an xdp_page, there is a potential > integer overflow in the buffer size test and trigger and out of bounds > memcpy(). > > Fix this by reordering the test so that both sides are of the same > signedness. > > Signed-off-by: Alexander Shishkin <alexander.shishkin at linux.intel.com> > Cc: Alexei Starovoitov <ast at kernel.org> > Cc: Daniel Borkmann <daniel at iogearbox.net> > Cc: Jesper Dangaard Brouer <hawk at kernel.org> > Cc: John Fastabend <john.fastabend at gmail.com> > Cc: David S. Miller <davem at davemloft.net> > Cc: Eric Dumazet <edumazet at google.com> > Cc: Jakub Kicinski <kuba at kernel.org> > Cc: Paolo Abeni <pabeni at redhat.com> > --- > drivers/net/virtio_net.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 7723b2a49d8e..dfa51dd95f63 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -751,8 +751,10 @@ static struct page *xdp_linearize_page(struct receive_queue *rq, > > /* guard against a misconfigured or uncooperative backend that > * is sending packet larger than the MTU. > + * At the same time, make sure that an especially uncooperative > + * backend can't overflow the test by supplying a large buflen. > */ > - if ((page_off + buflen + tailroom) > PAGE_SIZE) { > + if (buflen > PAGE_SIZE - page_off - tailroom) { > put_page(p); > goto err_buf; > }I feel the issue should be addressed in virtqueue_get_buf. In fact, when using DMA API, we already keep track of the length in vring_desc_extra. So, isn't this just the question of passing the length and validating it e.g. in vring_unmap_one_split? We can also use the index_nospec trick since otherwise there could be speculation concerns.> -- > 2.39.0