Re: [dpdk-dev] [PATCH] vhost: avoid buffer overflow in update_secure_len

["Xie, Huawei" <huawei.xie@intel.com>]

On 11/18/2015 11:53 PM, Stephen Hemminger wrote:
> On Wed, 18 Nov 2015 06:13:08 +0000
> "Xie, Huawei" <huawei.xie@intel.com> wrote:
>
>> On 11/18/2015 10:56 AM, Yuanhan Liu wrote:
>>> On Tue, Nov 17, 2015 at 08:39:30AM -0800, Rich Lane wrote:
>>>> I don't think that adding a SIGINT handler is the right solution, though. The
>>>> guest app could be killed with another signal (SIGKILL).
>>> Good point.
>>>
>>>> Worse, a malicious or
>>>> buggy guest could write to just that field. vhost should not crash no matter
>>>> what the guest writes into the virtqueues.
>> Rich, exactly, that has been in our list for a long time. We should
>> ensure that "Any malicious guest couldn't crash host through vrings"
>> otherwise this vhost implementation couldn't be deployed into production
>> environment.
>> There are many other known security holes in current dpdk vhost in my mind.
>> A very simple example is we don't check the gpa_to_vva return value, so
>> you could easily put a invalid GPA to vring entry to crash vhost.
>> My plan is to review the vhost implementation, fix all the possible
>> issues in one single patch set, and make the fix performance
>> optimization friendly rather than fix them here and there.
>>
> Both virtio and vhost need to adopt the "other side is broken" flag
> model that is in Linux drivers.  What this means is that the virtio
> and vhost driver would check parameters for consistency, and if out
> of bounds set a broken flag and refuse to do anything more with the
> device until reset.
Stephen:
You raise an important opinion.
Current DPDK virtio driver implementation chooses to trust the vhost, so
doesn't do any consistency check.
What is the reason that virtio driver also needs consistency check? Is
it that vhost might be buggy or that vhost might also not be trusted in
some user case?
/huawei
>