On Thu, Nov 4, 2021 at 1:07 PM Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> wrote: > > On Tue, Nov 2, 2021 at 3:04 PM Ioanna Alifieraki > <ioanna-maria.alifieraki@xxxxxxxxxxxxx> wrote: > > > > This is a v2 of the patches sent previously and hopefully makes things simpler. > > (previous patches subject: [PATCH 0/4] virt-aa-helper: Add new option to remove corrupted). > > > > This patch aims to address the bug reported in [1] and [2]. > > > > Bug description : > > Some times libvirt fails to start a vm with the following error : > > libvirt: error : unable to set AppArmor profile 'libvirt-b05b297f-952f-42d6-b04e-f9a13767db54' for '/usr/bin/kvm-spice': No such file or directory > > This happens because file /etc/apparmor.d/libvirt/libvirt-<vm-uuid> has 0 size. > > During the vm start-up virt-aa-helper tries to load the profile and because it is 0 it fails. > > When file /etc/apparmor.d/libvirt/libvirt-<vm-uuid> is removed the vm can start without problems. > > To address this issue this patch checks if the profile has 0 size and if this is > > the case it removes it. > > > > Changes with v1: > > I incorporated the feedback provided on v1 so the patches change as follows : > > > > Patches 1, 2 and 4 from v1 are dropped. > > The first patch is dropped because according to feedback provided remove_profile > > is not necessary and in the new version we unlink the profile directly in main(). > > In addition we skip calling create_profile twice by adding a boolean variable > > 'purged' if the profile was purged and creation occurs later on in main(). > > > > The second patch, which was adding a the option (-P) to remove the profile is dropped > > because currently this action happens only internally and there is no use case needed > > to make it available to the users of virt-aa-helper. > > > > The third patch which is the actual fix stays but modified. > > > > The forth patch which was adding a test to virt-aa-helper-test was the hardest to drop. > > Although, I'd like to have a test for this case, there is no apparent to make a test > > for this without having any side effects. > > The tests in virt-aa-helper-test are run with the --dryrun option and therefore no action > > should really happen. > > To test this fix, we need to create a corrupted profile and then remove it violating the dryrun. > > > > [1] https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1927519 > > [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890084 > > > > Ioanna Alifieraki (1): > > virt-aa-helper: Purge profile if corrupted > > Thanks to Jan for making this improved V2 happen. > > Slightly sad about not having a test, but since (as you explained) it > almost never would have ran there isn't much reason for it as-is. > > I was unsure at first if this now would have an issue when called with > -F triggering ctl->append extending the include_files and then (due to > empty profile setting purged) going into create_profile. > But since you only detect/refresh the profile, but not the > include_file that seems to work well even in that case. > > I appreciate your efforts on this! > Reviewed-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> > > With v7.9.0 tagged on Monday we are out of the freeze and I think we > can apply this unless there is any new negative feedback. FYI - I've ran another set of tests on it and since all were fine I applied it. > > src/security/virt-aa-helper.c | 20 +++++++++++++++++++- > > 1 file changed, 19 insertions(+), 1 deletion(-) > > > > -- > > 2.17.1 > > > > > -- > Christian Ehrhardt > Staff Engineer, Ubuntu Server > Canonical Ltd -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
