On Fri, Sep 20, 2013 at 12:29:31PM -0700, Steve Beattie wrote: > On Fri, Sep 20, 2013 at 12:26:11PM -0700, Steve Beattie wrote: > > On Wed, Sep 11, 2013 at 01:47:47AM -0700, Tyler Hicks wrote: > > > From: John Johansen <[email protected]> > > > > > > Convert the codomain to a class, and the policy lists that store > > > codomains to stl containers instead of glibc twalk. > > > > > > Signed-off-by: John Johansen <[email protected]> > > > [tyhicks: Merge with dbus changes and process_file_entries() cleanup] > > > Signed-off-by: Tyler Hicks <[email protected]> > > > > There's still a problem with this patch (even with the other fixes I > > made to it), it's not loading profiles with multiple hats properly: > > > > $ cat /tmp/example_profile > > /tests/regression/apparmor/changehat { > > > > ^sub { > > /proc/*/attr/current w, > > /tmp/sdtest.1713-15650-z0Mlub/file2 rw, > > } > > > > ^sub2 { > > /proc/*/attr/current w, > > /tmp/sdtest.1713-15650-z0Mlub/file2 rw, > > } > > > > ^sub3 { > > /proc/*/attr/current w, > > /tmp/sdtest.1713-15650-z0Mlub/file2 rw, > > } > > } > > > > $ sudo ./apparmor_parser /tmp/example_profile > > > > $ sudo grep changehat /sys/kernel/security/apparmor/profiles > > /tests/regression/apparmor/changehat (enforce) > > /tests/regression/apparmor/changehat//sub (enforce)
Alright, I traced it down and the following patch fixes it: Signed-off-by: Steve Beattie <[email protected]> --- parser/parser_interface.c | 2 +- parser/parser_policy.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Index: b/parser/parser_interface.c =================================================================== --- a/parser/parser_interface.c +++ b/parser/parser_interface.c @@ -837,7 +837,7 @@ int __sd_serialize_profile(int option, P close(fd); if (!prof->hat_table.empty() && option != OPTION_REMOVE) { - if (load_flattened_hats(prof, option) != 0) + if (load_flattened_hats(prof, option) == 0) return 0; } Index: b/parser/parser_policy.c =================================================================== --- a/parser/parser_policy.c +++ b/parser/parser_policy.c @@ -248,7 +248,7 @@ int load_policy_list(ProfileList &list, for (ProfileList::iterator i = list.begin(); i != list.end(); i++) { res = load_profile(option, *i); - if (!res) + if (res != 0) break; } The problem is a result of our sometimes returning non-zero to indicate errors and sometimes zero for errors. Tracking it down was made slightly more confusing because the original patch ended up creating two functions named sd_serialize_profile, with different parameters. While that's legal in C++ (name binding includes the argument types), it was confusing for this human to understand. Thus, I'd like to see something like the following as well (alternate suggestions for renaming accepted): Signed-off-by: Steve Beattie <[email protected]> --- parser/parser_interface.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Index: b/parser/parser_interface.c =================================================================== --- a/parser/parser_interface.c +++ b/parser/parser_interface.c @@ -60,7 +60,7 @@ #define SUBDOMAIN_INTERFACE_DFA_VERSION 5 -int sd_serialize_profile(int option, Profile *prof); +int __sd_serialize_profile(int option, Profile *prof); static void print_error(int error) { @@ -107,7 +107,7 @@ int load_profile(int option, Profile *pr int error = 0; PDEBUG("Serializing policy for %s.\n", prof->name); - retval = sd_serialize_profile(option, prof); + retval = __sd_serialize_profile(option, prof); if (retval < 0) { error = retval; /* yeah, we'll just report the last error */ @@ -700,7 +700,7 @@ int sd_serialize_top_profile(sd_serializ } int cache_fd = -1; -int sd_serialize_profile(int option, Profile *prof) +int __sd_serialize_profile(int option, Profile *prof) { int fd = -1; int error = -ENOMEM, size, wsize; -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
