Re: [Xen-devel] [RFC 1/4] libxl: Learned to send FD through QMP to QEMU

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 03/27/2018 11:29 AM, Ian Jackson wrote:
> (George, CC'ing you wrt your depriv doc patch - see below.)
> 
> Anthony PERARD writes ("[RFC 1/4] libxl: Learned to send FD through QMP to 
> QEMU"):
>> Adding the ability to send a file descriptor from libxl to QEMU via the
>> QMP interface. This will be use with the "add-fd" QMP command.
> 
> The code looks plausible.
> 
>> +    /* File descriptor to send to QEMU on the next command */
>> +    int fd_to_send;
> 
> I did wonder if this was a layering violation, or a poor API in some
> other sense.  AFAICT it isn't, and libxl__qmp_handler is completely
> transparent to everything in libxl_qmp.c.
> 
> I think this whole file would benefit from some doc comments about the
> internal interfaces.  Particularly, something describing the boundary
> between operation-specific code and the generic qmp_send machinery
> would help review of both (i) new operations and (ii) extensions of
> the generic machinery.
> 
> Looking at this and the next patch, I think (almost?) every user of
> this new feature will need to tell qmp_send to call
> qmp_fdset_add_fd_callback.  Is that right ?  Maybe this means we want
> to provide a more cooked version.
> 
> Anthony PERARD writes ("[RFC 2/4] libxl: Have QEMU save its state to a file 
> descriptor"):
>> In case QEMU have restricted access to the system, open the file for it,
>> and QEMU will save its state to this file descritor.
> 
> This 2nd patch looks reasonable, but it prompted to notice two new
> kinds of hazard introduced by the deprivileging design goal:
> 
>>  int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename, bool 
>> live)
>>  {
> ...
>> +    rc = qmp_synchronous_send(qmp, "add-fd", NULL,
>> +                              qmp_fdset_add_fd_callback, &new_fdset,
>> +                              qmp->timeout);
>> +    if (rc)
>> +        goto out;
> 
> By this point, a depriv'd qemu must be assumed to be compromised by
> its guest - ie we must treat it as hostile.
> 
> This is not consistent with use of qmp_synchronous_send, because
> qmp_synchronous_send will block with both the domain and ctx locks
> held.  That is, a malicious qemu can deny service; it even has the
> ability to prevent its serviced domain from being destroyed.

Will qmp_synchronous_send() wait forever, or is there a timeout?

In any case, we certainly do need to remember to treat QEMU as hostile
and audit the interactions with it.  This will help the stubdom case as
well.

I'll add it to the list.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel