On Mon, Mar 08, 2021 at 04:32:26PM +0100, Andrea Bolognani wrote: > On Mon, 2021-03-08 at 13:17 +0000, Daniel P. Berrangé wrote: > > On Mon, Mar 08, 2021 at 02:11:56PM +0100, Andrea Bolognani wrote: > > > The reason why VFIO device assignment is currently not completely > > > broken in KubeVirt is that, when the QEMU process is initially > > > started, we set the memory locking limit after fork(), so we can do > > > that using setrlimit() which doesn't require additional capabilities, > > > but in the hotplug scenario libvirtd needs to change the limits of a > > > different process: in that case we are forced to use prlimit(), which > > > fails due to the lack of CAP_SYS_RESOURCE. > > > > Since you added code to parse existing limits from /proc, I'm wondering > > if we can just do without the config option. Simply try to use prlimit > > and if it fails, query existing limits to determine if we sould treat > > the prlimit as fatal or ignore it. Overall I'd prefer libvirt to > > "just work" out of the box rather than requiring people to know about > > setting a "make-vfio-hotplug-work=yes" flag in the config file. > > The problem with that approach is what to do when *lowering* the > limit, for example as a consequence of hot-unplugging the last VFIO > device from the VM. > > If we're controlling the memory locking limit ourselves, then failure > to lower it should be an error, because leaving the limit much higher > than necessary creates potential for DoS by a compromised QEMU; on > the other hand, if the limit is controlled by an external process, > all we can really do is assume they will do the right thing after > hot-unplugging has happened.
IMHO once QEMU vCPUs start running, immediately assume QEMU is compromised / hostile. IOW, the DoS risk arrived the moment it was given the higher limit. We're just failing to close off the existing risk we've already accepted, which doesn't worry me much. On unplug the only thing we actually do when memory lock reduce fails is to log a warning message, it is never treated as a fatal error. So the only difference is whether we skip the warning message when we get EPERM from prlimit(), or always emit the warning. 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 :|
