Re: [libvirt] [PATCH] 0/10AppArmor driver updates
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
Attachments:
- signature.asc (application/pgp-signature — 198 bytes)