[apparmor] [patch] Prevent crash caused by by serialize_profile_from_old_profile()

[Seth Arnold]

On Tue, Dec 22, 2015 at 12:17:40AM +0100, Christian Boltz wrote:
> Therefore this patch wraps the serialize_profile_from_old_profile() call
> in try/except. If it fails, the diff will include an error message and
> recommend to use 'View Changes b/w (C)lean profiles' instead, which is
> known to work.

What does "b/w" mean? beziehungsweise?

> Note: I know using an error message as 'newprofile' isn't an usual way
> to display an error message, but I found it more intuitive than
> displaying it as a warning (without $PAGER).
> 
> 
> References: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
> 
> 
> I propose this patch for trunk and 2.10
> (2.9 "just" displays a wrong diff, but doesn't crash.)

Well, uh, begrudging ack for trunk and 2.10. Not-crashing is better than
crashing..

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks

> 
> 
> [ 43-prevent-crash-by-serialize_profile_from_old_profile.diff ]
> 
> === modified file ./utils/apparmor/aa.py
> --- utils/apparmor/aa.py        2015-12-21 00:13:57.215799543 +0100
> +++ utils/apparmor/aa.py        2015-12-22 00:04:28.306757169 +0100
> @@ -2368,7 +2368,12 @@
>                          oldprofile = aa[which][which]['filename']
>                      else:
>                          oldprofile = get_profile_filename(which)
> -                    newprofile = serialize_profile_from_old_profile(aa[which], which, '')
> +
> +                    try:
> +                        newprofile = serialize_profile_from_old_profile(aa[which], which, '')
> +                    except AttributeError:  
> +                        # see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
> +                        newprofile = "###\n###\n### Internal error while generating diff, please use '%s' instead\n###\n###\n" % _('View Changes b/w (C)lean profiles')
>  
>                      display_changes_with_comments(oldprofile, newprofile)
>  
> @@ -3638,6 +3643,11 @@
>      write_filelist = deepcopy(filelist[prof_filename])
>      write_prof_data = deepcopy(profile_data)
>  
> +    # XXX profile_data / write_prof_data contain only one profile with its hats
> +    # XXX this will explode if a file contains multiple profiles, see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
> +    # XXX fixing this needs lots of write_prof_data[hat] -> write_prof_data[profile][hat] changes (and of course also a change in the calling code)
> +    # XXX (the better option is a full rewrite of serialize_profile_from_old_profile())
> +
>      if options:  # and type(options) == dict:
>          if options.get('METADATA', False):
>              include_metadata = True
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160219/0670ea9e/attachment.pgp>