qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/5] backdoor: [softmmu] Add QEMU-side proxy to


From: Blue Swirl
Subject: Re: [Qemu-devel] [PATCH 4/5] backdoor: [softmmu] Add QEMU-side proxy to "libbackdoor.a"
Date: Fri, 30 Sep 2011 20:29:41 +0000

On Thu, Sep 29, 2011 at 10:35 PM, Frans de Boer <address@hidden> wrote:
> On 09/29/2011 11:49 PM, Lluís Vilanova wrote:
>>
>> Blue Swirl writes:
>>
>>> 2011/9/29 Lluís Vilanova<address@hidden>:
>>>>
>>>> +static uint64_t control_io_read(void *opaque, target_phys_addr_t addr,
>>>> unsigned size)
>>>> +{
>>>> +    State *s = opaque;
>>>> +
>>>> +    uint64_t res = ldq_p(&s->size);
>>>> +    uint8_t *resb = (uint8_t*)&res;
>>>> +    return resb[addr % CTRL_BYTES];
>>
>>> I don't think these lines do what you mean, but I'm also not sure what
>>> it is supposed to mean.
>>
>> Pre: only can read on a byte-per-byte basis (as stated in
>> control_ops.impl),
>> just because the code looks less ugly, and host performance should not be
>> an
>> issue here.
>>
>> The device is treated as a circular buffer of length CTRL_BYTES
>>
>> Reads are only used to get the size of the data channel.
>>
>> First line should handle guest/host endianess swapping, although I'm not
>> sure if
>> that's the API I'm supposed to use.
>>
>> Then return the N'th byte of the uint64_t variable holding the
>> (endianess-aware)
>> result.
>>
>>>> +}
>>>> +
>>>> +static void control_io_write(void *opaque, target_phys_addr_t addr,
>>>> uint64_t data, unsigned size)
>>>> +{
>>>> +    State *s = opaque;
>>>> +
>>>> +    uint8_t *cmdb = (uint8_t*)&s->cmd;
>>>> +    cmdb[addr % CTRL_BYTES] = (uint8_t)data;
>>>> +
>>>> +    if ((addr + size) % CTRL_BYTES == 0) {
>>>> +        qemu_backdoor(ldq_p(&s->cmd), s->data_ptr);
>>>> +    }
>>
>>> Same here.
>>
>> Pre: same as during reads.
>>
>> Accumulates writes into s->cmd to build the command the guest is sending
>> us (in
>> guest endianess).
>>
>> When CTRL_BYTES bytes have been written into the device, get the command
>> value
>> into host endianess and invoke the user-provided backdoor callback.
>>
>> This assumes that when executing in KVM, the device handling
>> infrastructure will
>> get a lock and only one CPU will be sending a backdoor command until
>> completion.
>>
>>
>> I'll add some comments there and prefix all structs and functions with
>> "backdoor_", as otherwise debugging could get harder if everyone started
>> avoiding the prefixes.
>>
>>
>> Lluis
>>
> I have not read the whole thread, but being an advisor in matters of
> information risk management, the very word "backdoor" send quivers along my
> spine. Why use a backdoor at all? The word alone gives us bad vibes.

Are the other methods for guest to interact with the host, like virtio
services OK then just because of their virtuous names? What about KVM,
isn't it horrible that untrusted guest code is executed by ultimately
trusted kernel module?

Jokes aside, the name could be improved.

> Using these words will disseminate QEMU from professional use since a
> possible risk might be introduced (even if it seems documented). So, please
> clarify the word "backdoor" or change it into something less threatening.

Obviously this device should never be enabled in any untrusted
environment, maybe not even compiled by default unless configured
during build. The same should apply to all debugging devices.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]