Re: [libvirt] [PATCH v3 04/13] security_manager: Rework metadata locking
On Wed, Oct 17, 2018 at 11:06:46AM +0200, Michal Privoznik wrote:
Trying to use virlockd to lock metadata turns out to be too big
gun. Since we will always spawn a separate process for relabeling
we are safe to use thread unsafe POSIX locks and take out
virtlockd completely out of the picture.
Note safety of POSIX locks is not merely about other threads.
If *any* code we invoke from security_dac or security_selinux
were to open() & close() the file we're labelling we'll
loose the locks.
This is a little concerning since we call out to a few functions
in src/util that might not be aware they should *never* open)(
and close() the path they're given. It looks like we lucky at
the moment, but this has future gun -> foot -> ouch potential.
Likewise we also call out to libselinux APIs. I've looked at
the code for the APIs we call and again, right now, we look
to be safe, but there's some risk there.
I'm not sure whether this is big enough risk to invalidate this
forking approach or not though. This kind of problem was the
reason why virtlockd was created to hold locks in a completely
separate process from the code with the critical section.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|