qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v8 7/7] virtio-console: Enable port throttling w


From: Paul Brook
Subject: Re: [Qemu-devel] [PATCH v8 7/7] virtio-console: Enable port throttling when chardev is slow to consume data
Date: Thu, 2 Dec 2010 17:31:36 +0000
User-agent: KMail/1.13.5 (Linux/2.6.36-trunk-amd64; KDE/4.4.5; x86_64; ; )

> > > when there's a partial write, it tries to do a write again, which will
> > > fail with -EAGAIN.
> > 
> > Doesn't that cause the first partial chunk to be incorrectly transmitted
> > twice? You may only return EAGAIN if no data was transmitted.
> 
> Except for the fact that no caller of qemu_chr_write() resubmits (or
> even checks) partial writes.

I don't buy this argument. The current implementation of qemu_chr_write never 
generates transient failures, so they don't need to.

If any data has been written then you must not return -EAGAIN.  Doing so will 
cause data corruption - the device will retry the transmit later (e.g. when 
the socket becomes unblocked) and duplicate data will be output.

Once data has been transmitted, we have three options:

a) Block until the write completes. This makes the whole patch fairly 
pointless as host and guest block boundaries are unlikely to align.

b) Store the data on the side somewhere. Tell the device all data has been 
sent, and arrange for this data to be flushed before accepting any more data.  
This is bad because it allows the guest to allocate arbitrarily large[1] 
buffers on the host. i.e. a fairly easily exploitable DoS attack.

c) Return a partial write to the guest. The guest already has to handle 
retries due to EAGAIN, and DMA capable devices already have to handle partial 
mappings, so this doesn't seem too onerous a requirement. This is not a new 
concept, it's the same as the unix write(2)/send(2) functions.

Paul

[1] At least as large as guest RAM, per port.



reply via email to

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