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@xxxxxxxxxxxxxxx> > Cc: Alexei Starovoitov <ast@xxxxxxxxxx> > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > Cc: Jesper Dangaard Brouer <hawk@xxxxxxxxxx> > Cc: John Fastabend <john.fastabend@xxxxxxxxx> > Cc: David S. Miller <davem@xxxxxxxxxxxxx> > Cc: Eric Dumazet <edumazet@xxxxxxxxxx> > Cc: Jakub Kicinski <kuba@xxxxxxxxxx> > Cc: Paolo Abeni <pabeni@xxxxxxxxxx> > --- > 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 Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization