[apparmor] [patch 03/26] fix failure paths around policy that can result in a crash

Seth Arnold seth.arnold at canonical.com
Tue Apr 15 23:48:27 UTC 2014


On Tue, Apr 15, 2014 at 10:22:10AM -0700, john.johansen at canonical.com wrote:
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> Acked-by: Steve Beattie <steve at nxnw.org>
> 

There's a lot of extra code duplication here. I don't particularly like
the way this thing turned out.. it's more obvious with the full code, I'll
paste it in below.

> ---
>  parser/parser_regex.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- 2.9-test.orig/parser/parser_regex.c
> +++ 2.9-test/parser/parser_regex.c
> @@ -712,6 +712,9 @@
>  		prof->policy.rules = NULL;
>  		if (!prof->policy.dfa)
>  			goto out;
> +	} else {
> +		aare_delete_ruleset(prof->policy.rules);
> +		prof->policy.rules = NULL;
>  	}
>  
>  	aare_reset_matchflags();
> @@ -719,6 +722,9 @@
>  	error = 0;
>  
>  out:
> +	aare_delete_ruleset(prof->policy.rules);
> +	prof->policy.rules = NULL;
> +
>  	return error;
>  }


# marks duplicated code
! marks a function that is called within the aare_delete_ruleset() function

I don't have a rewritten version handy to suggest, but this feels like
prime candidate for being cut in half.

        if (prof->policy.count > 0) {
                prof->policy.dfa = aare_create_dfa(prof->policy.rules,
                                                  &prof->policy.size,
                                                  dfaflags);
#               aare_delete_ruleset(prof->policy.rules);
#               prof->policy.rules = NULL;
                if (!prof->policy.dfa)
                        goto out;
        } else {
#               aare_delete_ruleset(prof->policy.rules);
#               prof->policy.rules = NULL;
        }

!       aare_reset_matchflags();

        error = 0;

out:
#       aare_delete_ruleset(prof->policy.rules);
#       prof->policy.rules = NULL;

        return error;
}


Thanks
-------------- 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/20140415/68de2aba/attachment-0001.pgp>


More information about the AppArmor mailing list