From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751719AbcDOLj6 (ORCPT ); Fri, 15 Apr 2016 07:39:58 -0400 Received: from e28smtp06.in.ibm.com ([125.16.236.6]:52523 "EHLO e28smtp06.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751014AbcDOLjy (ORCPT ); Fri, 15 Apr 2016 07:39:54 -0400 X-IBM-Helo: d28relay07.in.ibm.com X-IBM-MailFrom: zohar@linux.vnet.ibm.com X-IBM-RcptTo: linux-fsdevel@vger.kernel.org;linux-kernel@vger.kernel.org;linux-security-module@vger.kernel.org Message-ID: <1460720383.3256.188.camel@linux.vnet.ibm.com> Subject: Re: [RFC PATCH] fs: define a string representation of the kernel_read_file_id enumeration From: Mimi Zohar To: Kees Cook Cc: Al Viro , linux-security-module , linux-kernel , linux-fsdevel Date: Fri, 15 Apr 2016 07:39:43 -0400 In-Reply-To: References: <1460666952.3256.145.camel@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable x-cbid: 16041511-0021-0000-0000-00000B959F20 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-04-14 at 15:46 -0700, Kees Cook wrote: > On Thu, Apr 14, 2016 at 1:49 PM, Mimi Zohar 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 > > --- > > 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 > > > > >