On 2013-09-20 17:16:01, Steve Beattie wrote: > 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]>
This looks good to me.
> ---
> 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]>
> ---
I agree that it is a good idea to have different function names. I'm not
familiar enough with the code to suggest a better name right now, so
lets go with the __ prefix.
I'll roll both of these patches in.
Tyler
> 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/
> --
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
