[apparmor] [patch] Fix aa-mergeprof crash with files containing multiple profiles
apparmor at cboltz.de
Thu Feb 11 22:21:12 UTC 2016
Am Freitag, 12. Februar 2016, 03:26:36 CET schrieb Kshitij Gupta:
> On Sat, Dec 26, 2015, Christian Boltz <apparmor at cboltz.de> wrote:
> > if a profile file contains multiple profiles, aa-mergeprof crashes
> > on
> > saving in write_profile() because the second profile in the file is
> > not listed in 'changed'.
> > This patch first checks if 'changed' contains the profile before
> > pop()ing it.
> Assuming, this is because the second profile is actually not changing
Exactly. (I should have mentioned this in the patch description.)
> )looking at the in and out files, only comment was removed)/
> I guess ideally we should probably have two lists of profiles for a
> file(changed and unchanged), to maintain that the profile is always
> processed and we may show this info elsewhere.
Not really - the list of changed profiles should stay exactly what it is,
and we already make sure that we keep all profiles in a file.
One day, we'll need to support nested child profiles, and when we
implement that, we'll probably also have to adjust several of our
internal storage methods (aa[profile][hat] won't work anymore if you
allow deeper nesting, and several other things that use a similar
pattern will also explode. Oh, and external hats already explode with
the current code.)
While adjusting that, we can also address the file <-> profile
> > Reproducer: copy utils/test/cleanprof_test.in to your profile
> > directory and run aa-mergeprof utils/test/cleanprof_test.out.
> > Then just press 's' to save the profile.
> > I can reproduce this with trunk, 2.10 and 2.9 and therefore propose
> > this patch for all these branches.
> > [ 47-fix-multi-profile-mergeprof-crash.diff ]
> > --- utils/apparmor/aa.py 2015-12-26 16:47:30.614839586 +0100
> > +++ utils/apparmor/aa.py 2015-12-26 17:27:36.376228122 +0100
> > @@ -4039,7 +4039,11 @@
> > os.rename(newprof.name, prof_filename)
> > - changed.pop(profile)
> > + if profile in changed:
> > + changed.pop(profile)
> > + else:
> > + debug_logger.error("%s written, but not listed in 'changed'
> > list" % profile)
> > +
> We may rephrase it to something like: "Unchanged/unlisted profile %s
> written to file" with a log level of warn?
It will only be visible in the debug log (so it's unlikely someone sees
it in practise), but I'll change the message to
Unchanged profile written: %s
I won't switch to the warn loglevel because DebugLogger doesn't support
that ;-) - thinking about it, info is probably the better choice.
> With above nit-pick considered:
> Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>
> for trunk, 2.10 and 2.9
Wieviele Leute braucht es, um Windows zu installieren? - Sieben!
Einen, der sich die CD leisten kann, drei die ausdiskutieren, wie man
das wohl macht, zwei Priester, die beten, dass es funktioniert und einen
Psychiater, der die Nervenzusammenbrüche behandelt!!!
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 819 bytes
Desc: This is a digitally signed message part.
More information about the AppArmor