On Thu, 2016-04-14 at 15:46 -0700, Kees Cook wrote:
> On Thu, Apr 14, 2016 at 1:49 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> > (This patch is being posted as an RFC and has not been compiled.)
> >
> > A string representation of the kernel_read_file_id enumeration is needed
> > for displaying messages (eg. pr_info, auditing). We assume that the
> > string representation of the enumeration will be needed by multiple LSMs
> > and the integrity subsystem. Instead of each defining their own string
> > representation, this patch defines a common one.
> >
> > Each time a new enumeration entry is defined, it will need to be reflected
> > in the list of strings. To simplify keeping the list of strings in sync
> > with the enumeration, this patch proposes using two preprocessing
> > macros: stringify_1 and an a new macro named enumify.
> >
> > In general, preprocessing macros are not recommended. The question is
> > whether using preprocessing macros is preferable to having to remember to
> > update the list each time a new enumeration is defined.
> >
> > With these changes, the simplified version of kernel_read_file_id_str()
> > could be moved to a header.
> >
> > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>
> > ---
> > fs/exec.c | 28 ++++++++++++++--------------
> > include/linux/fs.h | 17 +++++++++++------
> > 2 files changed, 25 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/exec.c b/fs/exec.c
> > index 05e71b6..e9b9b85 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -819,25 +819,25 @@ struct file *open_exec(const char *name)
> > }
> > EXPORT_SYMBOL(open_exec);
> >
> > +static char *kernel_read_file_str[READING_MAX_ID];
> > const char *kernel_read_file_id_str(enum kernel_read_file_id id)
> > {
> > - switch (id) {
> > - case READING_FIRMWARE:
> > - return "firmware";
> > - case READING_MODULE:
> > - return "kernel-module";
> > - case READING_KEXEC_IMAGE:
> > - return "kexec-image";
> > - case READING_KEXEC_INITRAMFS:
> > - return "kexec-initramfs";
> > - case READING_POLICY:
> > - return "security-policy";
> > - default:
> > - return "unknown";
> > - }
> > + return kernel_read_file_str[id];
>
> (Whatever is decided, I'd still prefer an explicit bounds-check on the
> "id" argument here.)
Agreed.
> -Kees
Explicitly hard coding the strings, as you did, is clearer and easier to
read. It would be nice to get a general agreement as to whether using
macros in this case (and similar ones) is acceptable. (Cc'ing
linux-fsdevel)
Mimi
> > }
> > EXPORT_SYMBOL(kernel_read_file_id_str);
> >
> > +void __init kernel_read_file_init()
> > +{
> > + const char *kernel_read_file_upper_str[] = {
> > + __kernel_read_file_id(__stringify_1)
> > + };
> > +
> > + for (i = 0; i < READING_MAX_ID; i++) {
> > + kernel_read_file_str[i] = strdup(kernel_read_file_upper_str[i];
> > + lower_case(kernel_read_file_str[i];
> > + }
> > +}
> > +
> > int kernel_read(struct file *file, loff_t offset,
> > char *addr, unsigned long count)
> > {
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 23ea886..35ed80f 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2580,13 +2580,18 @@ static inline void i_readcount_inc(struct inode *inode)
> > #endif
> > extern int do_pipe_flags(int *, int);
> >
> > +#define __kernel_read_file_id(id) \
> > + id(UNKNOWN) \
> > + id(FIRMWARE) \
> > + id(MODULE) \
> > + id(KEXEC_IMAGE) \
> > + id(KEXEC_INITRAMFS) \
> > + id(POLICY) \
> > + id(MAX_ID) \
> > +#define __enumify(ENUM) READING_ ## ENUM,
> > +
> > enum kernel_read_file_id {
> > - READING_FIRMWARE = 1,
> > - READING_MODULE,
> > - READING_KEXEC_IMAGE,
> > - READING_KEXEC_INITRAMFS,
> > - READING_POLICY,
> > - READING_MAX_ID
> > + __kernel_read_file_id(__enumify)
> > };
> >
> > extern const char *kernel_read_file_id_str(enum kernel_read_file_id id);
> > --
> > 2.1.0
> >
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
