[Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Raphael Norwitz]

Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
and event without a physical address. This can cause
vhost_verify_ring_part_mapping to return ENOMEM, causing
the following logs:

qemu-system-x86_64: Unable to map available ring for ring 0
qemu-system-x86_64: Verify ring failure on region 0

The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
has already resolved the issue for vhost scsi devices but
the fix was never applied to vhost-user scsi devices.

Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 hw/scsi/vhost-user-scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index a9fd8ea..e4aae95 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -91,7 +91,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
     }
 
     vsc->dev.nvqs = 2 + vs->conf.num_queues;
-    vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs);
+    vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
     vsc->dev.vq_index = 0;
     vsc->dev.backend_features = 0;
     vqs = vsc->dev.vqs;
-- 
1.9.4

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
> Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
> and event without a physical address. This can cause
> vhost_verify_ring_part_mapping to return ENOMEM, causing
> the following logs:
> 
> qemu-system-x86_64: Unable to map available ring for ring 0
> qemu-system-x86_64: Verify ring failure on region 0
> 
> The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
> has already resolved the issue for vhost scsi devices but
> the fix was never applied to vhost-user scsi devices.
> 
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> ---
>  hw/scsi/vhost-user-scsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Raphael Norwitz]

On Fri, Jun 14, 2019 at 10:18:41AM +0100, Stefan Hajnoczi wrote:
> On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
> > Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
> > and event without a physical address. This can cause
> > vhost_verify_ring_part_mapping to return ENOMEM, causing
> > the following logs:
> > 
> > qemu-system-x86_64: Unable to map available ring for ring 0
> > qemu-system-x86_64: Verify ring failure on region 0
> > 
> > The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
> > has already resolved the issue for vhost scsi devices but
> > the fix was never applied to vhost-user scsi devices.
> > 
> > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> > ---
> >  hw/scsi/vhost-user-scsi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Ping on this. Any reason it has not been merged?

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Paolo Bonzini]

On 22/08/19 06:07, Raphael Norwitz wrote:
> On Fri, Jun 14, 2019 at 10:18:41AM +0100, Stefan Hajnoczi wrote:
>> On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
>>> Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
>>> and event without a physical address. This can cause
>>> vhost_verify_ring_part_mapping to return ENOMEM, causing
>>> the following logs:
>>>
>>> qemu-system-x86_64: Unable to map available ring for ring 0
>>> qemu-system-x86_64: Verify ring failure on region 0
>>>
>>> The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
>>> has already resolved the issue for vhost scsi devices but
>>> the fix was never applied to vhost-user scsi devices.
>>>
>>> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>>> ---
>>>  hw/scsi/vhost-user-scsi.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Ping on this. Any reason it has not been merged?

I had missed it.  I have now queued it (but I won't send another pull
request until approx. the middle of September).

Paolo

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Philippe Mathieu-Daudé]

[-- Attachment #1.1: Type: text/plain, Size: 945 bytes --]

Hi Raphael, Stefan,

On 6/14/19 11:18 AM, Stefan Hajnoczi wrote:
> On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
>> Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
>> and event without a physical address. This can cause
>> vhost_verify_ring_part_mapping to return ENOMEM, causing
>> the following logs:
>>
>> qemu-system-x86_64: Unable to map available ring for ring 0
>> qemu-system-x86_64: Verify ring failure on region 0
>>
>> The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
>> has already resolved the issue for vhost scsi devices but
>> the fix was never applied to vhost-user scsi devices.
>>
>> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>> ---
>>  hw/scsi/vhost-user-scsi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 

What about backends/vhost-user.c and hw/block/vhost-user-blk.c?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

On Wed, Aug 21, 2019 at 09:07:25PM -0700, Raphael Norwitz wrote:
> On Fri, Jun 14, 2019 at 10:18:41AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
> > > Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
> > > and event without a physical address. This can cause
> > > vhost_verify_ring_part_mapping to return ENOMEM, causing
> > > the following logs:
> > > 
> > > qemu-system-x86_64: Unable to map available ring for ring 0
> > > qemu-system-x86_64: Verify ring failure on region 0
> > > 
> > > The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
> > > has already resolved the issue for vhost scsi devices but
> > > the fix was never applied to vhost-user scsi devices.
> > > 
> > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> > > ---
> > >  hw/scsi/vhost-user-scsi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Ping on this. Any reason it has not been merged?

I think Paolo is away on vacation.  I will send it through my tree
instead.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Raphael Norwitz]

On Thu, Aug 22, 2019 at 10:58:02AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Raphael, Stefan,
> 
> On 6/14/19 11:18 AM, Stefan Hajnoczi wrote:
> > On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
> >> Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
> >> and event without a physical address. This can cause
> >> vhost_verify_ring_part_mapping to return ENOMEM, causing
> >> the following logs:
> >>
> >> qemu-system-x86_64: Unable to map available ring for ring 0
> >> qemu-system-x86_64: Verify ring failure on region 0
> >>
> >> The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
> >> has already resolved the issue for vhost scsi devices but
> >> the fix was never applied to vhost-user scsi devices.
> >>
> >> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> >> ---
> >>  hw/scsi/vhost-user-scsi.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> 
> What about backends/vhost-user.c and hw/block/vhost-user-blk.c?
> 

It is clearly an issue for hw/block/vhost-user-blk.c The code is very
similar.

backends/vhost-user.c also fails to zero out the vqs.

I will send the patches.