* [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets
@ 2019-07-22 13:24 Oleinik, Alexander
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size Oleinik, Alexander
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Oleinik, Alexander @ 2019-07-22 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, bsd, stefanha, Oleinik, Alexander
While fuzzing the virtio-net tx vq, I ran into an assertion failure due
to iov_copy offsets larger than the total iov size. Though there is
a check to cover this, it does not execute when !n->has_vnet_hdr. This
patch tries to fix this.
The call stack for the assertion failure:
#8 in __assert_fail (libc.so.6+0x300f1)
#9 in iov_copy iov.c:266:5
#10 in virtio_net_flush_tx virtio-net.c:2073:23
#11 in virtio_net_tx_bh virtio-net.c:2197:11
#12 in aio_bh_poll async.c:118:13
#13 in aio_dispatch aio-posix.c:460:5
#14 in aio_ctx_dispatch async.c:261:5
#15 in g_main_context_dispatch (libglib-2.0.so.0+0x4df2d)
#16 in glib_pollfds_poll main-loop.c:213:9
#17 in os_host_main_loop_wait main-loop.c:236
#18 in main_loop_wait main-loop.c:512
#19 in virtio_net_tx_fuzz virtio-net-fuzz.c:160:3
v2: add details to comment for the change to qemu_sendv_packet_async
Alexander Oleinik (2):
net: assert that tx packets have nonzero size
virtio-net: check that tx packet has positive size
hw/net/virtio-net.c | 15 +++++++++------
net/net.c | 9 +++++++++
2 files changed, 18 insertions(+), 6 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size
2019-07-22 13:24 [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Oleinik, Alexander
@ 2019-07-22 13:24 ` Oleinik, Alexander
2019-11-07 4:21 ` Jason Wang
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 2/2] virtio-net: check that tx packet has positive size Oleinik, Alexander
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Oleinik, Alexander @ 2019-07-22 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, bsd, Jason Wang, stefanha, Oleinik, Alexander
Virtual devices should not try to send zero-sized packets. The caller
should check the size prior to calling qemu_sendv_packet_async.
Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
v2:
* Improve the comment to explain the rationale for adding the assert.
net/net.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/net.c b/net/net.c
index 7d4098254f..4ad21df36f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -741,6 +741,15 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
size_t size = iov_size(iov, iovcnt);
int ret;
+ /*
+ * Since this function returns the size of the sent packets, and a return
+ * value of zero indicates that the packet will be sent asynchronously,
+ * there is currently no way to report that a 0-sized packet has been sent
+ * successfully. Forbid it for now, and if someone needs this functionality
+ * later, the API will require a change.
+ */
+ assert(size);
+
if (size > NET_BUFSIZE) {
return size;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] virtio-net: check that tx packet has positive size
2019-07-22 13:24 [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Oleinik, Alexander
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size Oleinik, Alexander
@ 2019-07-22 13:24 ` Oleinik, Alexander
2019-07-23 12:55 ` [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Stefan Hajnoczi
2019-11-06 15:33 ` Stefan Hajnoczi
3 siblings, 0 replies; 7+ messages in thread
From: Oleinik, Alexander @ 2019-07-22 13:24 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Jason Wang, Oleinik, Alexander, bsd,
stefanha, pbonzini
virtio_net_flush_tx does not check that the packet size is nonzero,
which causes q->aysnc_tx.elem to be set. Then, when the device is reset, there
is an assertion failure since q->aysnc_tx.elem must be flushed/cleared.
Zero-sized packets are unsupported - check packet size, prior to
sending.
Found while fuzzing virtio-net. The relevant stack-trace:
#8 in __assert_fail (libc.so.6+0x30101)
#9 in virtio_net_reset virtio-net.c:520:13
#10 in virtio_reset virtio.c:1214:9
#11 in virtio_pci_reset virtio-pci.c:1810:5
#12 in qdev_reset_one qdev.c:259:5
#13 in qdev_walk_children qdev.c:575:15
#14 in qbus_walk_children bus.c:52:15
#15 in qdev_walk_children qdev.c:567:15
#16 in qbus_walk_children bus.c:52:15
#17 in qemu_devices_reset reset.c:69:9
#18 in pc_machine_reset pc.c:2628:5
#19 in qemu_system_reset vl.c:1621:9
#20 in LLVMFuzzerTestOneInput fuzz.c:184:4
Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
hw/net/virtio-net.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b9e1cd71cf..743f1c827c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2074,12 +2074,15 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
out_sg = sg;
}
- ret = qemu_sendv_packet_async(qemu_get_subqueue(n->nic, queue_index),
- out_sg, out_num, virtio_net_tx_complete);
- if (ret == 0) {
- virtio_queue_set_notification(q->tx_vq, 0);
- q->async_tx.elem = elem;
- return -EBUSY;
+ /* Do not try to send 0-sized packets */
+ if (iov_size(out_sg, out_num)) {
+ ret = qemu_sendv_packet_async(qemu_get_subqueue(n->nic,
+ queue_index), out_sg, out_num, virtio_net_tx_complete);
+ if (ret == 0) {
+ virtio_queue_set_notification(q->tx_vq, 0);
+ q->async_tx.elem = elem;
+ return -EBUSY;
+ }
}
drop:
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets
2019-07-22 13:24 [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Oleinik, Alexander
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size Oleinik, Alexander
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 2/2] virtio-net: check that tx packet has positive size Oleinik, Alexander
@ 2019-07-23 12:55 ` Stefan Hajnoczi
2019-11-06 15:33 ` Stefan Hajnoczi
3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-07-23 12:55 UTC (permalink / raw)
To: Oleinik, Alexander; +Cc: pbonzini, bsd, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]
On Mon, Jul 22, 2019 at 01:24:44PM +0000, Oleinik, Alexander wrote:
> While fuzzing the virtio-net tx vq, I ran into an assertion failure due
> to iov_copy offsets larger than the total iov size. Though there is
> a check to cover this, it does not execute when !n->has_vnet_hdr. This
> patch tries to fix this.
> The call stack for the assertion failure:
>
> #8 in __assert_fail (libc.so.6+0x300f1)
> #9 in iov_copy iov.c:266:5
> #10 in virtio_net_flush_tx virtio-net.c:2073:23
> #11 in virtio_net_tx_bh virtio-net.c:2197:11
> #12 in aio_bh_poll async.c:118:13
> #13 in aio_dispatch aio-posix.c:460:5
> #14 in aio_ctx_dispatch async.c:261:5
> #15 in g_main_context_dispatch (libglib-2.0.so.0+0x4df2d)
> #16 in glib_pollfds_poll main-loop.c:213:9
> #17 in os_host_main_loop_wait main-loop.c:236
> #18 in main_loop_wait main-loop.c:512
> #19 in virtio_net_tx_fuzz virtio-net-fuzz.c:160:3
>
> v2: add details to comment for the change to qemu_sendv_packet_async
>
> Alexander Oleinik (2):
> net: assert that tx packets have nonzero size
> virtio-net: check that tx packet has positive size
>
> hw/net/virtio-net.c | 15 +++++++++------
> net/net.c | 9 +++++++++
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> --
> 2.20.1
>
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets
2019-07-22 13:24 [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Oleinik, Alexander
` (2 preceding siblings ...)
2019-07-23 12:55 ` [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Stefan Hajnoczi
@ 2019-11-06 15:33 ` Stefan Hajnoczi
3 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:33 UTC (permalink / raw)
To: jasowang, Michael S. Tsirkin
Cc: pbonzini, bsd, qemu-devel, stefanha, Oleinik, Alexander
[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]
On Mon, Jul 22, 2019 at 01:24:44PM +0000, Oleinik, Alexander wrote:
> While fuzzing the virtio-net tx vq, I ran into an assertion failure due
> to iov_copy offsets larger than the total iov size. Though there is
> a check to cover this, it does not execute when !n->has_vnet_hdr. This
> patch tries to fix this.
> The call stack for the assertion failure:
>
> #8 in __assert_fail (libc.so.6+0x300f1)
> #9 in iov_copy iov.c:266:5
> #10 in virtio_net_flush_tx virtio-net.c:2073:23
> #11 in virtio_net_tx_bh virtio-net.c:2197:11
> #12 in aio_bh_poll async.c:118:13
> #13 in aio_dispatch aio-posix.c:460:5
> #14 in aio_ctx_dispatch async.c:261:5
> #15 in g_main_context_dispatch (libglib-2.0.so.0+0x4df2d)
> #16 in glib_pollfds_poll main-loop.c:213:9
> #17 in os_host_main_loop_wait main-loop.c:236
> #18 in main_loop_wait main-loop.c:512
> #19 in virtio_net_tx_fuzz virtio-net-fuzz.c:160:3
>
> v2: add details to comment for the change to qemu_sendv_packet_async
>
> Alexander Oleinik (2):
> net: assert that tx packets have nonzero size
> virtio-net: check that tx packet has positive size
>
> hw/net/virtio-net.c | 15 +++++++++------
> net/net.c | 9 +++++++++
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> --
> 2.20.1
Ping
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size Oleinik, Alexander
@ 2019-11-07 4:21 ` Jason Wang
2019-11-21 13:51 ` Alexander Bulekov
0 siblings, 1 reply; 7+ messages in thread
From: Jason Wang @ 2019-11-07 4:21 UTC (permalink / raw)
To: Oleinik, Alexander, qemu-devel; +Cc: pbonzini, bsd, stefanha
On 2019/7/22 下午9:24, Oleinik, Alexander wrote:
> Virtual devices should not try to send zero-sized packets. The caller
> should check the size prior to calling qemu_sendv_packet_async.
>
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
> v2:
> * Improve the comment to explain the rationale for adding the assert.
> net/net.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/net.c b/net/net.c
> index 7d4098254f..4ad21df36f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -741,6 +741,15 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
> size_t size = iov_size(iov, iovcnt);
> int ret;
>
> + /*
> + * Since this function returns the size of the sent packets, and a return
> + * value of zero indicates that the packet will be sent asynchronously,
> + * there is currently no way to report that a 0-sized packet has been sent
> + * successfully. Forbid it for now, and if someone needs this functionality
> + * later, the API will require a change.
> + */
> + assert(size);
This probably will make the assertion triggerable from guest. Is this
better to warn and return NET_BUFSIZE + 1 here?
Thanks
> +
> if (size > NET_BUFSIZE) {
> return size;
> }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size
2019-11-07 4:21 ` Jason Wang
@ 2019-11-21 13:51 ` Alexander Bulekov
0 siblings, 0 replies; 7+ messages in thread
From: Alexander Bulekov @ 2019-11-21 13:51 UTC (permalink / raw)
To: Jason Wang; +Cc: pbonzini, bsd, qemu-devel, stefanha
On 191107 1221, Jason Wang wrote:
>
> On 2019/7/22 下午9:24, Oleinik, Alexander wrote:
> > Virtual devices should not try to send zero-sized packets. The caller
> > should check the size prior to calling qemu_sendv_packet_async.
> >
> > Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> > ---
> > v2:
> > * Improve the comment to explain the rationale for adding the assert.
> > net/net.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 7d4098254f..4ad21df36f 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -741,6 +741,15 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
> > size_t size = iov_size(iov, iovcnt);
> > int ret;
> > + /*
> > + * Since this function returns the size of the sent packets, and a return
> > + * value of zero indicates that the packet will be sent asynchronously,
> > + * there is currently no way to report that a 0-sized packet has been sent
> > + * successfully. Forbid it for now, and if someone needs this functionality
> > + * later, the API will require a change.
> > + */
> > + assert(size);
>
>
> This probably will make the assertion triggerable from guest. Is this better
> to warn and return NET_BUFSIZE + 1 here?
Will do. I'll send a v3 out with this change.
Thank you
> Thanks
>
>
> > +
> > if (size > NET_BUFSIZE) {
> > return size;
> > }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-21 13:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 13:24 [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Oleinik, Alexander
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 1/2] net: assert that tx packets have nonzero size Oleinik, Alexander
2019-11-07 4:21 ` Jason Wang
2019-11-21 13:51 ` Alexander Bulekov
2019-07-22 13:24 ` [Qemu-devel] [PATCH v2 2/2] virtio-net: check that tx packet has positive size Oleinik, Alexander
2019-07-23 12:55 ` [Qemu-devel] [PATCH v2 0/2] Avoid sending zero-size packets Stefan Hajnoczi
2019-11-06 15:33 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).