[dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Pavel Fedin]

Malfunctioning virtio clients may not send VHOST_USER_SET_MEM_TABLE for
some reason. This causes NULL dereference in qva_to_vva().

Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 lib/librte_vhost/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
index 0ba5045..3e7cec0 100644
--- a/lib/librte_vhost/virtio-net.c
+++ b/lib/librte_vhost/virtio-net.c
@@ -630,7 +630,7 @@ set_vring_addr(struct vhost_device_ctx ctx, struct vhost_vring_addr *addr)
 	struct vhost_virtqueue *vq;
 
 	dev = get_device(ctx);
-	if (dev == NULL)
+	if ((dev == NULL) || (dev->mem == NULL))
 		return -1;
 
 	/* addr->index refers to the queue index. The txq 1, rxq is 0. */
-- 
2.1.1

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Xie, Huawei]

On 1/13/2016 3:33 PM, Pavel Fedin wrote:
> Malfunctioning virtio clients may not send VHOST_USER_SET_MEM_TABLE for
> some reason. This causes NULL dereference in qva_to_vva().

Do you have examples for the malfunctioning clients? If so, could you
list them in the commit message?

>
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
>  lib/librte_vhost/virtio-net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 0ba5045..3e7cec0 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -630,7 +630,7 @@ set_vring_addr(struct vhost_device_ctx ctx, struct vhost_vring_addr *addr)
>  	struct vhost_virtqueue *vq;
>  
>  	dev = get_device(ctx);
> -	if (dev == NULL)
> +	if ((dev == NULL) || (dev->mem == NULL))
>  		return -1;
>  
>  	/* addr->index refers to the queue index. The txq 1, rxq is 0. */

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Pavel Fedin]

 Hello!

> Do you have examples for the malfunctioning clients? If so, could you
> list them in the commit message?

 The only malfunctioning client was DPDK itself, with virtio for container RFC applied. The client-side problem has been fixed
afterwards by http://dpdk.org/ml/archives/dev/2016-January/031169.html. See the RFC discussion thread.

Kind regards,
Pavel Fedin
Senior Engineer
Samsung Electronics Research center Russia

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Xie, Huawei]

On 1/13/2016 5:40 PM, Pavel Fedin wrote:
>  Hello!
>
>> Do you have examples for the malfunctioning clients? If so, could you
>> list them in the commit message?
>  The only malfunctioning client was DPDK itself, with virtio for container RFC applied. The client-side problem has been fixed
> afterwards by http://dpdk.org/ml/archives/dev/2016-January/031169.html. See the RFC discussion thread.

If this is the case, i am wondering whether we should include
"malfunctioning clients" in commit message. It triggers me to think if
there are existing buggy implementations.

Anyway, check is OK.

> Kind regards,
> Pavel Fedin
> Senior Engineer
> Samsung Electronics Research center Russia
>
>
>

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Pavel Fedin]

 Hello!

> If this is the case, i am wondering whether we should include
> "malfunctioning clients" in commit message. It triggers me to think if
> there are existing buggy implementations.

 Well... Can you suggest how to rephrase it? May be "if a client is malfunctioning it can..."? I lack fantasy, really, and to tell
the truth i don't care that much about the exact phrasing, i'm OK with everything.

> Anyway, check is OK.

Kind regards,
Pavel Fedin
Senior Engineer
Samsung Electronics Research center Russia

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Xie, Huawei]

On 1/15/2016 4:10 PM, Pavel Fedin wrote:
>  Hello!
>
>> If this is the case, i am wondering whether we should include
>> "malfunctioning clients" in commit message. It triggers me to think if
>> there are existing buggy implementations.
>  Well... Can you suggest how to rephrase it? May be "if a client is malfunctioning it can..."? I lack fantasy, really, and to tell
> the truth i don't care that much about the exact phrasing, i'm OK with everything.

Thanks.
Because there are not any existing malfunctioning clients which doesn't
send SET_MEM_TABLE message and we think the check is OK anyway,
personally I prefer simple phrases like: check memory table in
SET_VRING_ADDR message.

>
>> Anyway, check is OK.
> Kind regards,
> Pavel Fedin
> Senior Engineer
> Samsung Electronics Research center Russia
>
>
>

Re: [dpdk-dev] [RESEND PATCH] vhost_user: Make sure that memory map is set before attempting address translation

[Thomas Monjalon]

2016-01-13 10:32, Pavel Fedin:
> Malfunctioning virtio clients may not send VHOST_USER_SET_MEM_TABLE for
> some reason. This causes NULL dereference in qva_to_vva().
> 
> Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Applied, thanks