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 <[email protected]> > Cc: Alexei Starovoitov <[email protected]> > Cc: Daniel Borkmann <[email protected]> > Cc: Jesper Dangaard Brouer <[email protected]> > Cc: John Fastabend <[email protected]> > Cc: David S. Miller <[email protected]> > Cc: Eric Dumazet <[email protected]> > Cc: Jakub Kicinski <[email protected]> > Cc: Paolo Abeni <[email protected]> > --- > 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 _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
