On 15-11-09 09:30:58, Mimi Zohar wrote:
> On Mon, 2015-11-02 at 00:39 +0200, Petko Manolov wrote:
>
> > +
> > +#ifdef CONFIG_IMA_READ_POLICY
> > +enum {
> > + mask_err = -1,
> > + mask_exec = 1, mask_write, mask_read, mask_append
> > +};
> > +
> > +static match_table_t mask_tokens = {
> > + {mask_exec, "MAY_EXEC"},
> > + {mask_write, "MAY_WRITE"},
> > + {mask_read, "MAY_READ"},
> > + {mask_append, "MAY_APPEND"},
> > + {mask_err, NULL}
> > +};
> > +
> > +enum {
> > + func_err = -1,
> > + func_file = 1, func_mmap, func_bprm,
> > + func_module, func_firmware, func_post
> > +};
> > +
> > +static match_table_t func_tokens = {
> > + {func_file, "FILE_CHECK"},
> > + {func_mmap, "MMAP_CHECK"},
> > + {func_bprm, "BPRM_CHECK"},
> > + {func_module, "MODULE_CHECK"},
> > + {func_firmware, "FIRMWARE_CHECK"},
> > + {func_post, "POST_SETATTR"},
> > + {func_err, NULL}
> > +};
>
> Why are we using match_table_t? Why not define an array of strings which
> corresponds to the function hooks or use the __stringify macro?
Because you used match_table_t in your code. Having too many style differences
does not help it's readability.
> static const char *ima_hooks_string[] = {"", "FILE_CHECK", "MMAP_CHECK",
> "BPRM_CHECK", "MODULE_CHECK", "FIRMWARE_CHECK", "POST_SETATTR"};
>
> In the first case, to display the function hook string would be
> "ima_hooks_string[func]". Using __stringify requires the hook name (eg.
> __stringify(FILE_CHECK)).
>
> In either case, there would be a lot less code.
It is always speed vs size. It is going to be more code or more data. It is a
matter of taste which one we'll chose.
If we look at ima_policy.c after the original IMA policy read patch we'll end
up
with a source that is littered with strings like "fowner=%s", "fowner" and
"fowner=%d". I assumed you want this cleaned up, which i did.
Another example, "FILE_CHECK" was used just once prior to introducing of policy
read. Now we use it twice. Do we want to rely on GCC literals optimization
and
use the same string multiple times or hand-optimize it? What do we do with
almost the same strings like "fowner=%d", "fowner=%s" and "fowner"?
The answer to the above will determine whether we use an array of strings or
__stringify. I kind of lean towards the first option but as a maintainer the
choice is up to you.
Petko
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
