AppArmor

python aa-logprof: saving remaining profile crashes after "save selected profile"

Bug #1341178 reported by Christian Boltz
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Fix Released
Undecided
Unassigned
AppArmor 2.9.0

Bug Description

(bzr trunk from today)

When first using "save selected profile" and later using "save changes", aa-logprof crashes.

It looks like it removes the already written profile from the list of modified profiles only partially, and then tries to write it again (but fails because at least one part is already missing).

The following screen log shows what happens - I used "save selected profiles" twice before using "save changes".

# python3 aa-logprof
[...]
= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /usr/lib/boomaga/boomagamerger]
  2 - /sbin/dhclient
  3 - /usr/bin/boomaga
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t
Aktualisiertes Profil für /usr/lib/boomaga/boomagamerger wird geschrieben.

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /sbin/dhclient]
  2 - /usr/bin/boomaga
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /sbin/dhclient]
  2 - /usr/bin/boomaga
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t
Aktualisiertes Profil für /sbin/dhclient wird geschrieben.

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /usr/bin/boomaga]
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t

= Changed Local Profiles =

The following local profiles were changed. Would you like to save them?

 [1 - /usr/bin/boomaga]
(S)ave Changes / Save Selec(t)ed Profile / [(V)iew Changes] / View Changes b/w (C)lean profiles / Abo(r)t
Aktualisiertes Profil für /sbin/dhclient wird geschrieben.
Traceback (most recent call last):
  File "aa-logprof", line 52, in <module>
    apparmor.do_logprof_pass(logmark)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2291, in do_logprof_pass
    save_profiles()
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2386, in save_profiles
    write_profile_ui_feedback(profile_name)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 4327, in write_profile_ui_feedback
    write_profile(profile)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 4353, in write_profile
    changed.pop(profile)
KeyError: '/sbin/dhclient'

Revision history for this message
Christian Boltz (cboltz) wrote :

Note: aa.py has (line 2360):

                if ans == 'CMD_SAVE_SELECTED':
                    profile_name = list(changed.keys())[arg]
                    write_profile_ui_feedback(profile_name)
                    reload_base(profile_name)
                    #changed.pop(profile_name)
                    #q['options'] = changed

"changed.pop" was commented in r0.1.70:

                 if ans == 'CMD_SAVE_SELECTED':
                     profile_name = list(changed.keys())[arg]
                     write_profile_ui_feedback(profile_name)
                     reload_base(profile_name)
- changed.pop(profile_name)
+ #changed.pop(profile_name)
                     #q['options'] = changed

If I re-enable the "changed.pop" line, I get
    KeyError: '/usr/bin/boomaga'
for this line, so just removing the # is not an option.

Even more interesting: I added some debugging code:
                     #changed.pop(profile_name)
+ for tmp_prof_name in changed.keys():
+ print("*** changed.keys(): %s" % prof_name)
and it only prints the profiles that are not saved yet.

IIRC I used "save selected" and "save changes" in the past - could it be that python 3.4 introduced this issue? ("dictionary changed size during iteration" is also a new "feature" of this version...)

Revision history for this message
Christian Boltz (cboltz) wrote :

Found it - save_profiles does:

    changed_list = sorted(changed.keys())
[...]
                if ans == 'CMD_SAVE_SELECTED':
                    profile_name = list(changed.keys())[arg]
                    write_profile_ui_feedback(profile_name)
Note that write_profile_ui_feedback() calls write_profile(), which does "changed.pop(profile_name)"
[...]
            for profile_name in changed_list:
                write_profile_ui_feedback(profile_name)

In other words: when finally saving the remaining profiles, the outdated changed_list is used.

The solution is simple:
- for profile_name in changed_list:
+ for profile_name in sorted(changed.keys()):
                 write_profile_ui_feedback(profile_name)

I'll submit the patch to the ML ;-)

Revision history for this message
Christian Boltz (cboltz) wrote :

Fix commited to bzr trunk r2565.

Changed in apparmor:
status: New → Fix Committed
Steve Beattie (sbeattie)
Changed in apparmor:
milestone: none → 2.9.0
Revision history for this message
Steve Beattie (sbeattie) wrote :

Apparmor 2.9.0 has been released; closing.

Changed in apparmor:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.