[apparmor] patch - fix some bugs which appear to be the performance bug I was looking for

[Christian Boltz]

Hello,

Am Dienstag, 25. November 2014 schrieb Peter Maloney:
> Using the new hasher() from my previous patch, I found some problems.
> 
> One fix is here:
> 
> -                            if aa[profile][hat][incname]:
> +                            if
> aa[profile][hat]['include'].get(incname, False):
>                                  continue
> 
> I believe the old line would have created a new hasher (defaultdict)
> in the wrong place. The dict where it should have been needs to be
> fetched with the key 'include'. So this created lots of garbage in
> the aa[...] dict, plus would always return False, so the "continue"
> which was likely a performance optimization, was never called.

Indeed, nice catch!

> And another here:
> -                                    if
> aa[profile][hat]['allow']['path'][path].get('mode', False):
> -                                        mode |=
> aa[profile][hat]['allow']['path'][path]['mode']
> +                                    if path in
> aa[profile][hat]['allow']['path']:
> +                                        if
> aa[profile][hat]['allow']['path'][path].get('mode', False):
> +                                            mode |=
> aa[profile][hat]['allow']['path'][path]['mode']
> 
> And here:
> -        if include[incfile][incfile][allow]['path'][path]:
> +        if path in include[incfile][incfile][allow]['path']:
> 
> This one just prevents creating extra paths while looking them up. I
> believe this had a significant impact on performance, because it was
> going through more paths than it had to, to match against regexes, to
> find the modes, to " |= " them to a list. But the resulting dict was
> empty, so it would just add nothing and waste time.

Indeed.
 
> The first fix uses get(...,"False") because we want the value, which
> is either True or False. The others return strings or dicts. I don't
> understand why get('mode'), False) should be done instead of "if
> 'mode' in ..." but I left it the way it was since I think it works.

Those little changes bring a massive performance improvement - on my 
laptop, it did speedup aa-logprof (3.5 MB audit.log, without any 
questions asked) from 14 to 3.5 seconds :-)

Thanks for your patch!

For completeness, I should note that the patch also adds the filename in 
the "Can't find system log" exception. (I changed it back to '...' 
quotes.)


That said:
Acked-By: Christian Boltz <apparmor at cboltz.de>

I'll commit it to bzr in a minute.


Regards,

Christian Boltz
-- 
Personal Firewall bietet Schutz vor klassischen Angriffen -- Was hat
man unter "klassischen Angriffen" zu verstehen? -- Seltsam angezogene
Fremde rollen ein riesiges Holzpferd vor Deine Haustür...
[Johannes Sackmann, Heiko Schlenker, Ralf Bürckner]