DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address
@ 2015-03-26  7:04 Michael Qiu
  2015-03-26  7:52 ` Xie, Huawei
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Qiu @ 2015-03-26  7:04 UTC (permalink / raw)
  To: dev

Function gpa_to_vva() could return zero, while this will lead
a Segmentation fault.

This patch is to fix this issue.

Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
 lib/librte_vhost/vhost_rxtx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 535c7a1..23c8acb 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -587,6 +587,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
 
 		/* Buffer address translation. */
 		vb_addr = gpa_to_vva(dev, desc->addr);
+		if (!vb_addr)
+			return entry_success;
+
 		/* Prefetch buffer address. */
 		rte_prefetch0((void *)(uintptr_t)vb_addr);
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address
  2015-03-26  7:04 [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address Michael Qiu
@ 2015-03-26  7:52 ` Xie, Huawei
  2015-03-26  7:58   ` Qiu, Michael
  0 siblings, 1 reply; 5+ messages in thread
From: Xie, Huawei @ 2015-03-26  7:52 UTC (permalink / raw)
  To: Qiu, Michael; +Cc: dev

On 3/26/2015 3:05 PM, Qiu, Michael wrote:
> Function gpa_to_vva() could return zero, while this will lead
> a Segmentation fault.
>
> This patch is to fix this issue.
>
> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
> ---
>  lib/librte_vhost/vhost_rxtx.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
> index 535c7a1..23c8acb 100644
> --- a/lib/librte_vhost/vhost_rxtx.c
> +++ b/lib/librte_vhost/vhost_rxtx.c
> @@ -587,6 +587,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
>  
>  		/* Buffer address translation. */
>  		vb_addr = gpa_to_vva(dev, desc->addr);
> +		if (!vb_addr)
> +			return entry_success;
> +

Firstly we should add check for all gpa_to_vva translation, and do
reporting and cleanup on error. We should avoid the case that some buggy
or malicious guest virtio driver gives us an invalid GPA(for example,
GPA for some MMIO space) and crash our vhost process.

As we discuss, you meet segfault here, but our virtio PMD shouldn't give
us the GPA that has no translation, so we should root cause first and
fix the problem, and then submit the patch checking all gpa_to_vva
translation.

-Huawei
>  		/* Prefetch buffer address. */
>  		rte_prefetch0((void *)(uintptr_t)vb_addr);
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address
  2015-03-26  7:52 ` Xie, Huawei
@ 2015-03-26  7:58   ` Qiu, Michael
  2015-03-26  8:29     ` Linhaifeng
  0 siblings, 1 reply; 5+ messages in thread
From: Qiu, Michael @ 2015-03-26  7:58 UTC (permalink / raw)
  To: Xie, Huawei; +Cc: dev

On 3/26/2015 3:52 PM, Xie, Huawei wrote:
> On 3/26/2015 3:05 PM, Qiu, Michael wrote:
>> Function gpa_to_vva() could return zero, while this will lead
>> a Segmentation fault.
>>
>> This patch is to fix this issue.
>>
>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>> ---
>>  lib/librte_vhost/vhost_rxtx.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
>> index 535c7a1..23c8acb 100644
>> --- a/lib/librte_vhost/vhost_rxtx.c
>> +++ b/lib/librte_vhost/vhost_rxtx.c
>> @@ -587,6 +587,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
>>  
>>  		/* Buffer address translation. */
>>  		vb_addr = gpa_to_vva(dev, desc->addr);
>> +		if (!vb_addr)
>> +			return entry_success;
>> +
> Firstly we should add check for all gpa_to_vva translation, and do
> reporting and cleanup on error. We should avoid the case that some buggy
> or malicious guest virtio driver gives us an invalid GPA(for example,
> GPA for some MMIO space) and crash our vhost process.

Yes, agree, I will do this for next version.

> As we discuss, you meet segfault here, but our virtio PMD shouldn't give
> us the GPA that has no translation, so we should root cause first and

Yes, root cause is very important, but it will spend lots time, and I
think we could be possible to apply this first(All check version).

Thanks,
Michael
> fix the problem, and then submit the patch checking all gpa_to_vva
> translation.
>
> -Huawei
>>  		/* Prefetch buffer address. */
>>  		rte_prefetch0((void *)(uintptr_t)vb_addr);
>>  
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address
  2015-03-26  7:58   ` Qiu, Michael
@ 2015-03-26  8:29     ` Linhaifeng
  2015-03-27  8:45       ` Qiu, Michael
  0 siblings, 1 reply; 5+ messages in thread
From: Linhaifeng @ 2015-03-26  8:29 UTC (permalink / raw)
  To: Qiu, Michael, Xie, Huawei; +Cc: dev



On 2015/3/26 15:58, Qiu, Michael wrote:
> On 3/26/2015 3:52 PM, Xie, Huawei wrote:
>> On 3/26/2015 3:05 PM, Qiu, Michael wrote:
>>> Function gpa_to_vva() could return zero, while this will lead
>>> a Segmentation fault.
>>>
>>> This patch is to fix this issue.
>>>
>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>> ---
>>>  lib/librte_vhost/vhost_rxtx.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
>>> index 535c7a1..23c8acb 100644
>>> --- a/lib/librte_vhost/vhost_rxtx.c
>>> +++ b/lib/librte_vhost/vhost_rxtx.c
>>> @@ -587,6 +587,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
>>>  
>>>  		/* Buffer address translation. */
>>>  		vb_addr = gpa_to_vva(dev, desc->addr);
>>> +		if (!vb_addr)
>>> +			return entry_success;
>>> +
>> Firstly we should add check for all gpa_to_vva translation, and do
>> reporting and cleanup on error. We should avoid the case that some buggy
>> or malicious guest virtio driver gives us an invalid GPA(for example,
>> GPA for some MMIO space) and crash our vhost process.
> 
> Yes, agree, I will do this for next version.
> 
>> As we discuss, you meet segfault here, but our virtio PMD shouldn't give
>> us the GPA that has no translation, so we should root cause first and
> 
> Yes, root cause is very important, but it will spend lots time, and I
> think we could be possible to apply this first(All check version).
> 

How to deal with invalid address but not NULL?

> Thanks,
> Michael
>> fix the problem, and then submit the patch checking all gpa_to_vva
>> translation.
>>
>> -Huawei
>>>  		/* Prefetch buffer address. */
>>>  		rte_prefetch0((void *)(uintptr_t)vb_addr);
>>>  
>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address
  2015-03-26  8:29     ` Linhaifeng
@ 2015-03-27  8:45       ` Qiu, Michael
  0 siblings, 0 replies; 5+ messages in thread
From: Qiu, Michael @ 2015-03-27  8:45 UTC (permalink / raw)
  To: Linhaifeng, Xie, Huawei; +Cc: dev

On 3/26/2015 4:29 PM, Linhaifeng wrote:
>
> On 2015/3/26 15:58, Qiu, Michael wrote:
>> On 3/26/2015 3:52 PM, Xie, Huawei wrote:
>>> On 3/26/2015 3:05 PM, Qiu, Michael wrote:
>>>> Function gpa_to_vva() could return zero, while this will lead
>>>> a Segmentation fault.
>>>>
>>>> This patch is to fix this issue.
>>>>
>>>> Signed-off-by: Michael Qiu <michael.qiu@intel.com>
>>>> ---
>>>>  lib/librte_vhost/vhost_rxtx.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
>>>> index 535c7a1..23c8acb 100644
>>>> --- a/lib/librte_vhost/vhost_rxtx.c
>>>> +++ b/lib/librte_vhost/vhost_rxtx.c
>>>> @@ -587,6 +587,9 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
>>>>  
>>>>  		/* Buffer address translation. */
>>>>  		vb_addr = gpa_to_vva(dev, desc->addr);
>>>> +		if (!vb_addr)
>>>> +			return entry_success;
>>>> +
>>> Firstly we should add check for all gpa_to_vva translation, and do
>>> reporting and cleanup on error. We should avoid the case that some buggy
>>> or malicious guest virtio driver gives us an invalid GPA(for example,
>>> GPA for some MMIO space) and crash our vhost process.
>> Yes, agree, I will do this for next version.
>>
>>> As we discuss, you meet segfault here, but our virtio PMD shouldn't give
>>> us the GPA that has no translation, so we should root cause first and
>> Yes, root cause is very important, but it will spend lots time, and I
>> think we could be possible to apply this first(All check version).
>>
> How to deal with invalid address but not NULL?

The problem is how do you know if it is a valid addres?

Thanks,
Michael
>
>> Thanks,
>> Michael
>>> fix the problem, and then submit the patch checking all gpa_to_vva
>>> translation.
>>>
>>> -Huawei
>>>>  		/* Prefetch buffer address. */
>>>>  		rte_prefetch0((void *)(uintptr_t)vb_addr);
>>>>  
>>
>>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-03-27  8:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26  7:04 [dpdk-dev] [PATCH] vhost: Fix Segmentation fault of NULL address Michael Qiu
2015-03-26  7:52 ` Xie, Huawei
2015-03-26  7:58   ` Qiu, Michael
2015-03-26  8:29     ` Linhaifeng
2015-03-27  8:45       ` Qiu, Michael

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git