[Qemu-devel] [PATCH 1/1] Fix loop logic in vhost_net_start()'s error path

[Qemu-devel] [PATCH 1/1] Fix loop logic in vhost_net_start()'s error path

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

file.index is unsigned, hence 'while (--file.index >= 0)' will loop
forever. Change it to do {} while (file.index-- > 0)

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 hw/vhost_net.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 4a7b819..b1f8072 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -151,10 +151,10 @@ int vhost_net_start(struct vhost_net *net,
     return 0;
 fail:
     file.fd = -1;
-    while (--file.index >= 0) {
+    do {
         int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
         assert(r >= 0);
-    }
+    } while (file.index-- > 0);
     net->vc->info->poll(net->vc, true);
     vhost_dev_stop(&net->dev, dev);
     if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
-- 
1.7.2.2

[Qemu-devel] Re: [PATCH 1/1] Fix loop logic in vhost_net_start()'s error path

[Michael S. Tsirkin]

On Tue, Sep 14, 2010 at 12:06:12PM +0200, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> file.index is unsigned, hence 'while (--file.index >= 0)' will loop
> forever. Change it to do {} while (file.index-- > 0)
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  hw/vhost_net.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vhost_net.c b/hw/vhost_net.c
> index 4a7b819..b1f8072 100644
> --- a/hw/vhost_net.c
> +++ b/hw/vhost_net.c
> @@ -151,10 +151,10 @@ int vhost_net_start(struct vhost_net *net,
>      return 0;
>  fail:
>      file.fd = -1;
> -    while (--file.index >= 0) {
> +    do {
>          int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
>          assert(r >= 0);
> -    }
> +    } while (file.index-- > 0);
>      net->vc->info->poll(net->vc, true);
>      vhost_dev_stop(&net->dev, dev);
>      if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {

Hmm, this is not exactly right in that we first try with
file.index and not file.index - 1; if SET_BACKEND originally failed
with file.index and we try to undo this, we are likely to fail again
and trigger an assert.

I fixed this as follows.
Thanks!



vhost: fix infinite loop on error path

file.index is unsigned, hence 'while (--file.index >= 0)'
will loop > forever. Change to while (file.index-- > 0).

Reported-by: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

--

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 606aa0c..f2f4740 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -139,7 +139,7 @@ int vhost_net_start(struct vhost_net *net,
     return 0;
 fail:
     file.fd = -1;
-    while (--file.index >= 0) {
+    while (file.index-- > 0) {
         int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
         assert(r >= 0);
     }