Top posting since this is only a reminder... I was wondering if this could be re-reviewed now that 0.8 is out. Thanks! Jamie On Tue, 2010-04-06 at 16:56 -0500, Jamie Strandboge wrote: > On Tue, 2010-04-06 at 23:05 +0200, Daniel Veillard wrote: > > On Mon, Apr 05, 2010 at 04:15:06PM -0500, Jamie Strandboge wrote: > > > This patch series addresses bug fixes in the AppArmor driver as well as > > > updating it to changes made in 0.7.6 and 0.7.7. All of these are > > > self-contained within the driver except 4_qemu_driver_stdin_path.patch. > > > This is required by 5_apparmor-fix-save-restore.patch (see below). These > > > all pass 'make syntax-check' and 'make check' (except 'daemon-conf', > > > which has never passed here and I didn't patch it). > > [...] > > > > > > 4_qemu_driver_stdin_path.patch: adjust args to qemudStartVMDaemon() to > > > also specify path to stdin_fd, so this can be passed to the AppArmor > > > driver via *SetSecurityAllLabel(). This updates all calls to > > > qemudStartVMDaemon() as well as setting up the non-AppArmor security > > > driver *SetSecurityAllLabel() declarations for the above. This is > > > required for 5_apparmor-fix-save-restore.patch since AppArmor resolves > > > the passed file descriptor to the pathname given to open(). > > > > > > 5_apparmor-fix-save-restore.patch: refactoring to update AppArmor > > > security driver to adjust profile for save/restore[3] > > > > Okay, I have now pushed all the patches except 4/ and 5/ which were > > changing some of the internal secutity layers of the qemud and it sounds > > a bit late in the cycle for this, plus I would prefer some review from > > Dan before pushing like this, > > > > Just for a little more background on '4': qemudStartVMDaemon() passes > stdin_fd to virExecDaemonize(), which launches qemu passing it the file > descriptor if it is not -1. qemudDomainRestore() is the only place where > stdin_fd is a valid file descriptor (not -1). However, when qemu is > launched with this fd, the kernel resolves this to the real path and > checks to see if the process has access to this path, and the restore > fails because this path is not in the AppArmor profile. Since there is > no easy/portable way to obtain a pathname based on a file descriptor, we > need to pass the path name given to open() (when we got the stdin_fd) to > qemudStartVMDaemon() so that we can then turn around and give it to > driver->securityDriver->domainSetSecurityAllLabel(). This way the > AppArmor security driver can update the profile using the pathname. > > In my patch, qemudStartVMDaemon() is updated to have a new argument: > stdin_path. All calls to qemudStartVMDaemon() add NULL for this added > argument, except the one call in qemudDomainRestore(), which uses > 'path' (the pathname given to open() to obtain 'fd'). > qemudStartVMDaemon() is updated to pass stdin_path to > domainSetSecurityAllLabel(). security_driver.h, qemu_security_dac.c, > qemu_security_stacked.c and security_selinux.c are adjusted to have the > extra argument in the *Set*AllLabel functions, with the drivers all > using ATTRIBUTE_UNUSED. > > '5' adjusts the apparmor driver to implement domainSetSavedStateLabel > and domainRestoreSavedStateLabel based on the above, and also has some > refactoring for code reuse by these functions. > > > > but 1-3, 7-10 are in :-) > > Actually, it is 1-3 and 6-10 that were committed. Thanks for the review > and commits! :) -- Jamie Strandboge | http://www.canonical.com
Attachment:
signature.asc
Description: This is a digitally signed message part
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list