Re: [libvirt] [PATCH v2 01/11] security: Always spawn process for transactions

On 10/12/18 4:45 AM, Michal Privoznik wrote: Considering the number of fork's out there anyway, that expense wasn't part of my consideration. It's a means to an end. If someone disabled Namespace's for whatever reason, then how would this path work? virProcessRunInMountNamespace => virProcessNamespaceHelper (child) I get that it's "just" a fork, but my point is more that the namespace is Namespace and if that's not true, then isn't the premise of the patch not true? And no IDC about non Linux (where Namespace wouldn't be available) since non Linux wouldn't have DAC and/or SELinux either. Perhaps the light dawning over marblehead moment for me is that the security driver will use Namespaces similar to how the QEMU driver uses them, but instead in the context of whatever process/thread that isn't necessarily the domain process (in this case the libvirtd process). Since it's been proven that the code works for the QEMU domain processing, ergo it should "just work" for the libvirtd security driver processing. That's very different from the commit message above and perhaps the "assumption" I've been thinking under. There isn't a "relationship" between the qemu.conf usage of namespaces and what's being done here except for the libvirtd process as opposed to the qemu-* process will use fork's and Namespaces. Something perhaps for both the commit message *and* at least the two modules that change here to more clearly describe the @pid usage. The virSecurityManagerTransactionCommit will need adjustment too in order to indicate that the underlying code will run in the Namespace specified by the caller rather than the Namespace of the caller (a really vague, but important difference). Well disabling isn't really well described in the qemu.conf description, but I'm fairly certain it's by uncommenting the entry and removing the "mount" from the [ ], leaving just "namespaces = [ ]" For some more data points using the previously failing guest that now works with the alteration below. Running with the "default": # nsenter -m -t $(pgrep qemu) ls -l /dev | wc -l 16 # ls -l /dev | wc -l 269 # By changing to "namespaces = [ ]", I get: # nsenter -m -t $(pgrep qemu) ls -l /dev | wc -l 269 # ls -l /dev | wc -l 269 # Which I'm not sure I expected, but maybe that's right. Next, if I run without the changes in this series and adding a couple of debug messages to show when virProcessNamespaceHelper is called, when namespaces = [ ] there are no calls to to the helper; whereas, there are two using the default (I assume one each for DAC and SELinux). With this series applied, when namespaces = [ ] there are 8 calls to to the helper (all from the "parent" gdb pid); whereas, for the default the last 2 come from the domain. I assume that's all expected (at this point). As I note above, I think I've had the light dawning moment now. I think prior to this point I've been under the impression that the QEMU domain setting for namespaces was somehow "affecting" or "related to" what's being done/changed here. In reality it's making use or reusing the same code that QEMU uses. Still the one reservation is - what happens if setns isn't available. For that I think you need to somehow decide that it isn't available and just pass -1, unless you have some other grand idea. Whether you build in some "disable" switch just in case is up to you. You feel confident that things will just work and I can agree, but you know there are pessimists that may think otherwise. John