[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] 0/10AppArmor driver updates



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]