[Qemu-devel] Re: [PATCH 5/5] virtio-serial: Error out if guest sends unexpected vq elements

2010-12-10 Thread Amit Shah
On (Fri) Dec 10 2010 [13:59:50], Paul Brook wrote:
  Check if the guest really sent any items in the out_vq before using
  them.  Similarly, check if there is a buffer to send data in before
  writing.
 
 Can this actually happen? If so why/how?
 Why does it need a special case in this device?

A malicious guest (ie, a guest with the virtio_console module suitably
modified) could send in buffers with the 'input' bit set instead of
output as expected or vice-versa.

 If this is guest triggerable then calling abort() is wrong.

It's either a guest bug or a malicious guest.  What action is
recommended?

Amit



[Qemu-devel] Re: [PATCH 5/5] virtio-serial: Error out if guest sends unexpected vq elements

2010-12-10 Thread Amit Shah
On (Fri) Dec 10 2010 [15:17:58], Paul Brook wrote:
  On (Fri) Dec 10 2010 [13:59:50], Paul Brook wrote:
Check if the guest really sent any items in the out_vq before using
them.  Similarly, check if there is a buffer to send data in before
writing.
   
   Can this actually happen? If so why/how?
   Why does it need a special case in this device?
  
  A malicious guest (ie, a guest with the virtio_console module suitably
  modified) could send in buffers with the 'input' bit set instead of
  output as expected or vice-versa.
 
 So what? Who cares if they get it wrong?

Just let the error_report() be there and continue as if nothing
happened?

 It's entirely unclear whether this is actually an error. If a request has 
 zero 
 size then we just transfer zero bytes, exactly as requested.
 
 Even if you accept this should be a diagnosable error, I suspect your patch 
 is 
 still insufficient. I don't see any code to check that input queue requests 
 have zero output segments, nor do I see anything to handle zero-length 
 segments.

virtio actually supports sending both, input as well as output types of
buffers in one go.

   If this is guest triggerable then calling abort() is wrong.
  
  It's either a guest bug or a malicious guest.  What action is
  recommended?
 
 Killing the whole VM in response to a malformed request to a device is 
 clearly 
 a bug in that device.  You should report an error to the guest in the normal 
 manner.  IIRC virtio lacks any consistent error reporting mechanisms, and the 
 usual response when asked to do something impossible is to reset the device.

OK, agreed.

Amit