[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] xen: add pvUSB backend
From: |
Juergen Gross |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] xen: add pvUSB backend |
Date: |
Fri, 6 May 2016 06:57:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 |
On 05/05/16 12:13, Anthony PERARD wrote:
> On Wed, May 04, 2016 at 10:25:03AM +0200, Juergen Gross wrote:
>> On 03/05/16 17:06, Anthony PERARD wrote:
>>> On Thu, Mar 10, 2016 at 04:19:30PM +0100, Juergen Gross wrote:
>>>> +static void usbback_bh(void *opaque)
>>>> +{
>>>> + struct usbback_info *usbif;
>>>> + struct usbif_urb_back_ring *urb_ring;
>>>> + struct usbback_req *usbback_req;
>>>> + RING_IDX rc, rp;
>>>> + unsigned int more_to_do;
>>>> +
>>>> + usbif = opaque;
>>>> + if (usbif->ring_error) {
>>>> + return;
>>>> + }
>>>> +
>>>> + urb_ring = &usbif->urb_ring;
>>>> + rc = urb_ring->req_cons;
>>>> + rp = urb_ring->sring->req_prod;
>>>
>>> Maybe use atomic_read() here to avoid req_prod been read more than once.
>>
>> Hmm. This isn't done in the other backends.
>>
>> TBH: what would happen if req_prod would be read multiple times? In the
>> worst case we would see a new request from the guest which we would have
>> missed without the atomic_read().
>
> If the guest is misbehaving, it maybe could provoke QEMU to handle more
> request. I'm not sure.
I don't think this would add any risk to dom0. A misbehaving guest
writing arbitrary values to ->req_prod could influence qemu activity
in just the same way regardless whether atomic_read() is used on qemu
side or not. The only difference would be that with atomic_read() the
additional qemu activity would be delayed until the next invocation
of the function.
> For this use of atomic_read, I'm mostly refering to XSA-155[1] and a
> conversation[2].
The main problem with XSA-155 was modification of the request's contents
by the guest after verification by the backend happened. This is not
related to reading the producer's ring index.
I should use RING_COPY_REQUEST(), however.
Juergen