On 10/19/2015 11:15 AM, Dmitry Vyukov wrote:
> On Mon, Oct 19, 2015 at 5:08 PM, Sasha Levin <[email protected]> wrote:
>> > On 10/19/2015 10:47 AM, Dmitry Vyukov wrote:
>>>> >>> Right, the memory areas that are accessed both by the hypervisor and
>>>> >>> the guest
>>>>> >>> > should be treated as untrusted input, but the hypervisor is
>>>>> >>> > supposed to validate
>>>>> >>> > the input carefully before using it - so I'm not sure how data
>>>>> >>> > races would
>>>>> >>> > introduce anything new that we didn't catch during validation.
>>> >>
>>> >> One possibility would be: if result of a racy read is passed to guest,
>>> >> that can leak arbitrary host data into guest. Does not sound good.
>>> >> Also, without usage of proper atomic operations, it is basically
>>> >> impossible to verify untrusted data, as it can be changing under your
>>> >> feet. And storing data into a local variable does not prevent the data
>>> >> from changing.
>> >
>> > What's missing here is that the guest doesn't directly read/write the
>> > memory:
>> > every time it accesses a memory that is shared with the host it will
>> > trigger
>> > an exit, which will stop the vcpu thread that made the access and kernel
>> > side
>> > kvm will pass the hypervisor the value the guest wrote (or the memory
>> > address
>> > it attempted to read). The value/address can't change under us in that
>> > scenario.
> But still: if result of a racy read is passed to guest, that can leak
> arbitrary host data into guest.
I see what you're saying. I need to think about it a bit, maybe we do need
locking
for each of the virtio devices we emulate.
On an unrelated note, a few of the reports are pointing to ioport__unregister():
==================
WARNING: ThreadSanitizer: data race (pid=109228)
Write of size 8 at 0x7d1c0000df40 by main thread:
#0 free tsan/rtl/tsan_interceptors.cc:570 (lkvm+0x000000443376)
#1 ioport__unregister ioport.c:138:2 (lkvm+0x0000004a9ff9)
#2 pci__exit pci.c:247:2 (lkvm+0x0000004ac857)
#3 init_list__exit util/init.c:59:8 (lkvm+0x0000004bca6e)
#4 kvm_cmd_run_exit builtin-run.c:645:2 (lkvm+0x0000004a68a7)
#5 kvm_cmd_run builtin-run.c:661 (lkvm+0x0000004a68a7)
#6 handle_command kvm-cmd.c:84:8 (lkvm+0x0000004bc40c)
#7 handle_kvm_command main.c:11:9 (lkvm+0x0000004ac0b4)
#8 main main.c:18 (lkvm+0x0000004ac0b4)
Previous read of size 8 at 0x7d1c0000df40 by thread T55:
#0 rb_int_search_single util/rbtree-interval.c:14:17 (lkvm+0x0000004bf968)
#1 ioport_search ioport.c:41:9 (lkvm+0x0000004aa05f)
#2 kvm__emulate_io ioport.c:186 (lkvm+0x0000004aa05f)
#3 kvm_cpu__emulate_io x86/include/kvm/kvm-cpu-arch.h:41:9
(lkvm+0x0000004aa718)
#4 kvm_cpu__start kvm-cpu.c:126 (lkvm+0x0000004aa718)
#5 kvm_cpu_thread builtin-run.c:174:6 (lkvm+0x0000004a6e3e)
Thread T55 'kvm-vcpu-2' (tid=109285, finished) created by main thread at:
#0 pthread_create tsan/rtl/tsan_interceptors.cc:848 (lkvm+0x0000004478a3)
#1 kvm_cmd_run_work builtin-run.c:633:7 (lkvm+0x0000004a683f)
#2 kvm_cmd_run builtin-run.c:660 (lkvm+0x0000004a683f)
#3 handle_command kvm-cmd.c:84:8 (lkvm+0x0000004bc40c)
#4 handle_kvm_command main.c:11:9 (lkvm+0x0000004ac0b4)
#5 main main.c:18 (lkvm+0x0000004ac0b4)
SUMMARY: ThreadSanitizer: data race ioport.c:138:2 in ioport__unregister
==================
I think this is because we don't perform locking using pthread, but rather pause
the vm entirely - so the cpu threads it's pointing to aren't actually running
when
we unregister ioports. Is there a way to annotate that for tsan?
Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
