[Qemu-devel] [PATCH] vhost: fix invalid downcast

[Qemu-devel] [PATCH] vhost: fix invalid downcast

[Yury Kotov]

virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 32-bit.
If returned hwaddr is not equal to 0 but least-significant 32 bits are
equal to 0 then this code will not actually stop running queue.

Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
---
 hw/virtio/vhost.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index b129cb9..7edeee7 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1071,10 +1071,8 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
         .index = vhost_vq_index,
     };
     int r;
-    int a;
 
-    a = virtio_queue_get_desc_addr(vdev, idx);
-    if (a == 0) {
+    if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
         /* Don't stop the virtqueue which might have not been started */
         return;
     }
-- 
2.7.4

Re: [Qemu-devel] [PATCH] vhost: fix invalid downcast

[Michael S. Tsirkin]

On Fri, Jul 13, 2018 at 05:04:05PM +0300, Yury Kotov wrote:
> virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 32-bit.
> If returned hwaddr is not equal to 0 but least-significant 32 bits are
> equal to 0 then this code will not actually stop running queue.
> 
> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>

So IIUC

Fixes: fb20fbb764aa1 ("vhost: avoid to start/stop virtqueue which is not ready")
And 
Cc: qemu-stable@nongnu.org

> ---
>  hw/virtio/vhost.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index b129cb9..7edeee7 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1071,10 +1071,8 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>          .index = vhost_vq_index,
>      };
>      int r;
> -    int a;
>  
> -    a = virtio_queue_get_desc_addr(vdev, idx);
> -    if (a == 0) {
> +    if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
>          /* Don't stop the virtqueue which might have not been started */
>          return;
>      }
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH] vhost: fix invalid downcast

[Jia He]

On 7/14/2018 12:15 AM, Michael S. Tsirkin Wrote:
> On Fri, Jul 13, 2018 at 05:04:05PM +0300, Yury Kotov wrote:
>> virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 32-bit.
>> If returned hwaddr is not equal to 0 but least-significant 32 bits are
>> equal to 0 then this code will not actually stop running queue.
>>
>> Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
> 
> So IIUC
> 
> Fixes: fb20fbb764aa1 ("vhost: avoid to start/stop virtqueue which is not ready")

> And 
> Cc: qemu-stable@nongnu.org
> 
>> ---
>>  hw/virtio/vhost.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index b129cb9..7edeee7 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1071,10 +1071,8 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>>          .index = vhost_vq_index,
>>      };
>>      int r;
>> -    int a;
>>  
>> -    a = virtio_queue_get_desc_addr(vdev, idx);
>> -    if (a == 0) {
>> +    if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
>>          /* Don't stop the virtqueue which might have not been started */
>>          return;
>>      }
>> -- 
>> 2.7.4
> 
yes, it is a bug introduced by fb20fbb764aa1
Acked-by: Jia He <hejianet@gmail.com>
-- 
Cheers,
Jia

Re: [Qemu-devel] [PATCH] vhost: fix invalid downcast

[Юрий Котов]

+ qemu-stable@

13.07.2018, 19:16, "Michael S. Tsirkin" <mst@redhat.com>:
> On Fri, Jul 13, 2018 at 05:04:05PM +0300, Yury Kotov wrote:
>>  virtio_queue_get_desc_addr returns 64-bit hwaddr while int is usually 32-bit.
>>  If returned hwaddr is not equal to 0 but least-significant 32 bits are
>>  equal to 0 then this code will not actually stop running queue.
>>
>>  Signed-off-by: Yury Kotov <yury-kotov@yandex-team.ru>
>
> So IIUC
>
> Fixes: fb20fbb764aa1 ("vhost: avoid to start/stop virtqueue which is not ready")
> And
> Cc: qemu-stable@nongnu.org
>

Ok, done. Or did you mean I have to resend the patch-message to qemu-stable?

>>  ---
>>   hw/virtio/vhost.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>>  diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>  index b129cb9..7edeee7 100644
>>  --- a/hw/virtio/vhost.c
>>  +++ b/hw/virtio/vhost.c
>>  @@ -1071,10 +1071,8 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>>           .index = vhost_vq_index,
>>       };
>>       int r;
>>  - int a;
>>
>>  - a = virtio_queue_get_desc_addr(vdev, idx);
>>  - if (a == 0) {
>>  + if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
>>           /* Don't stop the virtqueue which might have not been started */
>>           return;
>>       }
>>  --
>>  2.7.4