Re: [Xen-devel] [PATCH 9/9] libxl: Kill QEMU with "reaper" ruid

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Nov 28, 2018, at 6:33 PM, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
> 
>>> -        ret = setresuid(dm_uid, dm_uid, 0);
>>> +        fd = open(lockfile, O_RDWR|O_CREAT, 0666);
>>> +        if (fd < 0) {
>>> +            /* All other errno: EBADF, EINVAL, ENOLCK, EWOULDBLOCK */
>>> +            LOGED(ERROR, domid,
>>> +                  "unexpected error while trying to open lockfile %s, 
>>> errno=%d",
>>> +                  lockfile, errno);
>>> +            goto kill;
>> 
>> More gotos!  I doubt this error handling is right.
>> 
>> I'm also not convinced that it is sensible to handle the case where we
>> have multiple per-domain uids but no reaper uid.  This turns host
>> configuration errors into systems that are less secure than they
>> should be in a really obscure way.
> 
> At this point, we have a target_uid but no way of getting a lock for 
> reaper_uid.  We have three options:
> 
> 1. Don’t kill(-1) at all.
> 2. Try to  kill(-1) with setresuid(target_uid, target_uid, 0)
> 3. kill(-1) with setresuid(reaper_uid, target_uid, 0) without holding the 
> lock.
> 
> #1 means that a rogue qemu will not be destroyed.
> 
> #2 means that there’s a race, whereby sometimes the rogue qemu is destroyed, 
> and sometimes the reaper process is destroyed by the rogue qemu first.
> 
> #3 means there’s a race, whereby sometimes everything works fine, sometimes 
> both the rogue qemu and *the reaper process from another domain* is 
> destroyed, and sometimes this reaper process is killed by the reaper process 
> from another domain (leaving the rogue qemu alive).
> 
> I think #1 is obviously the best option.

Sorry, this should say, ‘I think #2 is the best option’.

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