On Tue, 2006-07-25 at 10:55 -0400, Joshua Brindle wrote:
> The setools team would like to be able to optionally expand neverallow
> rules for analysis purposes. This patch leaves the current behavior
> unchanged, but allows a new state variable for the expander to indicate
> whether neverallow rules should get expanded, and creates an init
> function for the expand_state struct.

How much memory does it take to expand the neverallow rules for a typical reference policy? Can you post some hashtable stats - this might make the chains very long and hurt performance for looking up non-neverallow rules. These tradeoffs may be fine for analysis, but it would be nice to have some comments explaining the effects for other users of the library.

<snip>

> +static void expand_state_init(expand_state_t *state)
> +{
> + memset(state, 0, sizeof(expand_state_t));
> +}
> +

I assume you've audited everywhere expand is currently called to use this function?

<snip>

> @@ -1200,7 +1210,7 @@ static int expand_rule_helper(sepol_hand
> if (!ebitmap_node_get_bit(snode, i))
> continue;
> if (source_rule->flags & RULE_SELF) {
> - if (source_rule->specified & AVRULE_AV) {
> + if (source_rule->specified & (AVRULE_AV|AVRULE_NEVERALLOW)) {
> if ((retval =
> expand_avrule_helper(handle,
> source_rule->
> @@ -1227,7 +1237,7 @@ static int expand_rule_helper(sepol_hand
> ebitmap_for_each_bit(ttypes, tnode, j) {
> if (!ebitmap_node_get_bit(tnode, j))
> continue;
> - if (source_rule->specified & AVRULE_AV) {
> + if (source_rule->specified & (AVRULE_AV|AVRULE_NEVERALLOW)) {
> if ((retval =
> expand_avrule_helper(handle,
> source_rule->

It might be cleaner to have a new define with AVRULE_AV and AVRULE_NEVERALLOW already or'd. I don't feel strongly about it though.

> @@ -1264,13 +1274,14 @@ static int convert_and_expand_rule(sepol
> policydb_t * dest_pol, uint32_t * typemap,
> avrule_t * source_rule, avtab_t * dest_avtab,
> cond_av_list_t ** cond,
> - cond_av_list_t ** other, int enabled)
> + cond_av_list_t ** other, int enabled,
> + int do_neverallow)
> {
> int retval;
> ebitmap_t stypes, ttypes;
> unsigned char alwaysexpand;
>
> - if (source_rule->specified & AVRULE_NEVERALLOW)
> + if (!do_neverallow && source_rule->specified & AVRULE_NEVERALLOW)
> return 1;
>

Magic return code.

> ebitmap_init(&stypes);
> @@ -1306,7 +1317,7 @@ static int cond_avrule_list_copy(policyd
> while (cur) {
> if (convert_and_expand_rule(state->handle, dest_pol,
> typemap, cur, dest_avtab,
> - list, other, enabled) != 1) {
> + list, other, enabled, 0) != 1) {
> return -1;
> }
>
> @@ -1897,6 +1908,8 @@ int expand_module(sepol_handle_t * handl
> expand_state_t state;
> avrule_block_t *curblock;
>
> + expand_state_init(&state);
> +
> state.verbose = verbose;
> state.typemap = NULL;
> state.base = base;
> @@ -2033,7 +2046,7 @@ int expand_module(sepol_handle_t * handl
> /* copy rules */
> cur_avrule = decl->avrules;
> while (cur_avrule != NULL) {
> - if (cur_avrule->specified & AVRULE_NEVERALLOW) {
> + if (!(state->expand_neverallow) && cur_avrule->specified & AVRULE_NEVERALLOW) {

I think that the copying of the neverallows needs to be factored out completely - it is a strange side effect of expansion. The expansion functions should just, well, expand rules.

Karl

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.