Message ID | 20190719185158.20316-2-alxndr@bu.edu |
---|---|
State | New |
Headers | show |
Series | Avoid sending zero-size packets | expand |
On Fri, Jul 19, 2019 at 06:52:24PM +0000, 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> > --- > net/net.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/net.c b/net/net.c > index 7d4098254f..fad20bc611 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -741,6 +741,9 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender, > size_t size = iov_size(iov, iovcnt); > int ret; > > + /* 0-sized packets are unsupported. Check size in the caller */ > + assert(size); Please include the rationale: A return value of 0 means the packet has been queued and will be sent asynchronously. Therefore this function has no way of reporting that a 0-sized packet has been sent successfully. Forbid it for now and if someone needs this functionality then the API will require a change. This way someone who hits this will understand why the assertion is there. Stefan
On 2019/7/20 上午2:52, 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> > --- > net/net.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/net.c b/net/net.c > index 7d4098254f..fad20bc611 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -741,6 +741,9 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender, > size_t size = iov_size(iov, iovcnt); > int ret; > > + /* 0-sized packets are unsupported. Check size in the caller */ > + assert(size); Can this be triggered through a buggy device by guest? If yes, we need avoid using assert() here. Thanks > + > if (size > NET_BUFSIZE) { > return size; > }
On Tue, 2019-07-23 at 11:38 +0800, Jason Wang wrote: > On 2019/7/20 上午2:52, 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> > > --- > > net/net.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/net/net.c b/net/net.c > > index 7d4098254f..fad20bc611 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -741,6 +741,9 @@ ssize_t qemu_sendv_packet_async(NetClientState > > *sender, > > size_t size = iov_size(iov, iovcnt); > > int ret; > > > > + /* 0-sized packets are unsupported. Check size in the caller > > */ > > + assert(size); > > Can this be triggered through a buggy device by guest? If yes, we > need > avoid using assert() here. Yes - for example, virtio-net could trigger trigger it because there is no size check prior to qemu_sendv_packet_async, although PATCH 2/2 should fix this. Instead of the assertion, should we return a negative value? > Thanks > > > > + > > if (size > NET_BUFSIZE) { > > return size; > > }
diff --git a/net/net.c b/net/net.c index 7d4098254f..fad20bc611 100644 --- a/net/net.c +++ b/net/net.c @@ -741,6 +741,9 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender, size_t size = iov_size(iov, iovcnt); int ret; + /* 0-sized packets are unsupported. Check size in the caller */ + assert(size); + if (size > NET_BUFSIZE) { return size; }
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> --- net/net.c | 3 +++ 1 file changed, 3 insertions(+)