> On Nov 28, 2018, at 6:33 PM, George Dunlap <[email protected]> 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 [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
