On Tue, 2015-11-10 at 18:01 +0200, Petko Manolov wrote:
> 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.
We're using match_token() for parsing the policy, which requires
match_table_t.
> > 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.
>From what I've seen playing with __stringify, it has its own drawbacks.
I would probably use an enumeration of strings arrays and remove the
switch/case statements as much as possible.
Mimi
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
