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

Re: [PATCH] libs/light: make it build without setresuid()

Manuel Bouyer writes ("Re: [PATCH] libs/light: make it build without 
> On Wed, Jan 20, 2021 at 02:52:06PM +0000, Ian Jackson wrote:
> > I don't think setuid is safe - at least, if we are trying to restrict
> > the dm.  Since I think after the libxl child is forked, and has called
> What is the dm in this case ? qemu ? On NetBSD qemu runs as root AFAIK,
> so there isn't much to protect.

Yes, the dm is qemu.  If qemu restriction is not supported, that makes
a big difference.  The complex situation here is to do with trying to
kill a possibly hostile qemu.

> > setuid, it might be traceable (by NetBSD's equivalent of ptrace) by
> > the dm.  The dm could puppet it into pretending it had succeeded, but
> > then hang around until the domid is reused.
> I don't understand. We're talking about a simple kill(2) syscall here.

If we're not trying to restrict qemu's privilege at all, then I think
the setuid is fine.  There are then only two remaining concerns I have
with this patch:

Firstly, we try to avoid #ifdefs like this.  It tends to make the code
rather tangled, especially over time.  Instead we prefer to move the
non-portable code into its own file, eg *_linux.c.

Secondly, I think we should check that dm_restrict is not enabled.
I think an assert would do since I think we believe this is already
prevented elsewhere ?

(One option for making this work would be to simply disable the
killing by uid on NetBSD.  But I don't think that's a good answer
because killing by uid after eg setuid is more reliable even if it is
not 100% bulletproof.  So switching to setuid or maybe setreuid is the
right answer.)

> OK so if I understand properly, you say Xen should not be used on NetBSD ?

I'm sorry to have offended and discouraged you.  That was not my
intention.  My apologies for sending an off-putting message.  For the
avoidance of any doubt, definitely don't think that.  We should make
this work properly.

Would you be willing to look into the two points I mention above and
send a revised version of the patch ?  If you find the refactoring
awkward I or Roger can help.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.