Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933035Ab0BDAfe (ORCPT ); Wed, 3 Feb 2010 19:35:34 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:40031 "EHLO e9.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932991Ab0BDAfc (ORCPT ); Wed, 3 Feb 2010 19:35:32 -0500 Date: Wed, 3 Feb 2010 18:35:28 -0600 From: "Serge E. Hallyn" To: Kees Cook Cc: James Morris , Casey Schaufler , linux-security-module@vger.kernel.org, Eric Paris , David Howells , Alexey Dobriyan , Ingo Molnar , Andrew Morton , Simon Kagstrom , David Woodhouse , Robin Getz , Greg Kroah-Hartman , Paul Moore , Tetsuo Handa , Stephen Smalley , Etienne Basset , "David P. Quigley" , LKLM Subject: Re: [PATCH v2 2/2] syslog: use defined constants instead of raw numbers Message-ID: <20100204003528.GB16681@us.ibm.com> References: <20100202055354.GV19355@outflux.net> <4B67C2EA.705@schaufler-ca.com> <20100202202054.GW19355@outflux.net> <20100202212510.GG32305@us.ibm.com> <20100203233713.GJ19355@outflux.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100203233713.GJ19355@outflux.net> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9032 Lines: 255 Quoting Kees Cook (kees.cook@canonical.com): > Right now the syslog "type" action are just raw numbers which makes > the source difficult to follow. This patch replaces the raw numbers > with defined constants for some level of sanity. > > Signed-off-by: Kees Cook Acked-by: Serge Hallyn > --- > fs/proc/kmsg.c | 10 +++++----- > include/linux/syslog.h | 23 +++++++++++++++++++++++ > kernel/printk.c | 45 +++++++++++++++++++-------------------------- > security/commoncap.c | 5 +++-- > security/selinux/hooks.c | 21 +++++++++++---------- > 5 files changed, 61 insertions(+), 43 deletions(-) > > diff --git a/fs/proc/kmsg.c b/fs/proc/kmsg.c > index 6a3d843..cfe90a4 100644 > --- a/fs/proc/kmsg.c > +++ b/fs/proc/kmsg.c > @@ -21,12 +21,12 @@ extern wait_queue_head_t log_wait; > > static int kmsg_open(struct inode * inode, struct file * file) > { > - return do_syslog(1, NULL, 0, SYSLOG_FROM_FILE); > + return do_syslog(SYSLOG_ACTION_OPEN, NULL, 0, SYSLOG_FROM_FILE); > } > > static int kmsg_release(struct inode * inode, struct file * file) > { > - (void) do_syslog(0, NULL, 0, SYSLOG_FROM_FILE); > + (void) do_syslog(SYSLOG_ACTION_CLOSE, NULL, 0, SYSLOG_FROM_FILE); > return 0; > } > > @@ -34,15 +34,15 @@ static ssize_t kmsg_read(struct file *file, char __user *buf, > size_t count, loff_t *ppos) > { > if ((file->f_flags & O_NONBLOCK) && > - !do_syslog(9, NULL, 0, SYSLOG_FROM_FILE)) > + !do_syslog(SYSLOG_ACTION_SIZE_UNREAD, NULL, 0, SYSLOG_FROM_FILE)) > return -EAGAIN; > - return do_syslog(2, buf, count, SYSLOG_FROM_FILE); > + return do_syslog(SYSLOG_ACTION_READ, buf, count, SYSLOG_FROM_FILE); > } > > static unsigned int kmsg_poll(struct file *file, poll_table *wait) > { > poll_wait(file, &log_wait, wait); > - if (do_syslog(9, NULL, 0, SYSLOG_FROM_FILE)) > + if (do_syslog(SYSLOG_ACTION_SIZE_UNREAD, NULL, 0, SYSLOG_FROM_FILE)) > return POLLIN | POLLRDNORM; > return 0; > } > diff --git a/include/linux/syslog.h b/include/linux/syslog.h > index 5f02b18..3891139 100644 > --- a/include/linux/syslog.h > +++ b/include/linux/syslog.h > @@ -21,6 +21,29 @@ > #ifndef _LINUX_SYSLOG_H > #define _LINUX_SYSLOG_H > > +/* Close the log. Currently a NOP. */ > +#define SYSLOG_ACTION_CLOSE 0 > +/* Open the log. Currently a NOP. */ > +#define SYSLOG_ACTION_OPEN 1 > +/* Read from the log. */ > +#define SYSLOG_ACTION_READ 2 > +/* Read all messages remaining in the ring buffer. */ > +#define SYSLOG_ACTION_READ_ALL 3 > +/* Read and clear all messages remaining in the ring buffer */ > +#define SYSLOG_ACTION_READ_CLEAR 4 > +/* Clear ring buffer. */ > +#define SYSLOG_ACTION_CLEAR 5 > +/* Disable printk's to console */ > +#define SYSLOG_ACTION_CONSOLE_OFF 6 > +/* Enable printk's to console */ > +#define SYSLOG_ACTION_CONSOLE_ON 7 > +/* Set level of messages printed to console */ > +#define SYSLOG_ACTION_CONSOLE_LEVEL 8 > +/* Return number of unread characters in the log buffer */ > +#define SYSLOG_ACTION_SIZE_UNREAD 9 > +/* Return size of the log buffer */ > +#define SYSLOG_ACTION_SIZE_BUFFER 10 > + > #define SYSLOG_FROM_CALL 0 > #define SYSLOG_FROM_FILE 1 > > diff --git a/kernel/printk.c b/kernel/printk.c > index 1771b34..4067412 100644 > --- a/kernel/printk.c > +++ b/kernel/printk.c > @@ -259,21 +259,6 @@ static inline void boot_delay_msec(void) > } > #endif > > -/* > - * Commands to do_syslog: > - * > - * 0 -- Close the log. Currently a NOP. > - * 1 -- Open the log. Currently a NOP. > - * 2 -- Read from the log. > - * 3 -- Read all messages remaining in the ring buffer. > - * 4 -- Read and clear all messages remaining in the ring buffer > - * 5 -- Clear ring buffer. > - * 6 -- Disable printk's to console > - * 7 -- Enable printk's to console > - * 8 -- Set level of messages printed to console > - * 9 -- Return number of unread characters in the log buffer > - * 10 -- Return size of the log buffer > - */ > int do_syslog(int type, char __user *buf, int len, bool from_file) > { > unsigned i, j, limit, count; > @@ -286,11 +271,11 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) > return error; > > switch (type) { > - case 0: /* Close log */ > + case SYSLOG_ACTION_CLOSE: /* Close log */ > break; > - case 1: /* Open log */ > + case SYSLOG_ACTION_OPEN: /* Open log */ > break; > - case 2: /* Read from log */ > + case SYSLOG_ACTION_READ: /* Read from log */ > error = -EINVAL; > if (!buf || len < 0) > goto out; > @@ -321,10 +306,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) > if (!error) > error = i; > break; > - case 4: /* Read/clear last kernel messages */ > + /* Read/clear last kernel messages */ > + case SYSLOG_ACTION_READ_CLEAR: > do_clear = 1; > /* FALL THRU */ > - case 3: /* Read last kernel messages */ > + /* Read last kernel messages */ > + case SYSLOG_ACTION_READ_ALL: > error = -EINVAL; > if (!buf || len < 0) > goto out; > @@ -377,21 +364,25 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) > } > } > break; > - case 5: /* Clear ring buffer */ > + /* Clear ring buffer */ > + case SYSLOG_ACTION_CLEAR: > logged_chars = 0; > break; > - case 6: /* Disable logging to console */ > + /* Disable logging to console */ > + case SYSLOG_ACTION_CONSOLE_OFF: > if (saved_console_loglevel == -1) > saved_console_loglevel = console_loglevel; > console_loglevel = minimum_console_loglevel; > break; > - case 7: /* Enable logging to console */ > + /* Enable logging to console */ > + case SYSLOG_ACTION_CONSOLE_ON: > if (saved_console_loglevel != -1) { > console_loglevel = saved_console_loglevel; > saved_console_loglevel = -1; > } > break; > - case 8: /* Set level of messages printed to console */ > + /* Set level of messages printed to console */ > + case SYSLOG_ACTION_CONSOLE_LEVEL: > error = -EINVAL; > if (len < 1 || len > 8) > goto out; > @@ -402,10 +393,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file) > saved_console_loglevel = -1; > error = 0; > break; > - case 9: /* Number of chars in the log buffer */ > + /* Number of chars in the log buffer */ > + case SYSLOG_ACTION_SIZE_UNREAD: > error = log_end - log_start; > break; > - case 10: /* Size of the log buffer */ > + /* Size of the log buffer */ > + case SYSLOG_ACTION_SIZE_BUFFER: > error = log_buf_len; > break; > default: > diff --git a/security/commoncap.c b/security/commoncap.c > index 677fad9..cf01b2e 100644 > --- a/security/commoncap.c > +++ b/security/commoncap.c > @@ -897,9 +897,10 @@ error: > int cap_syslog(int type, bool from_file) > { > /* /proc/kmsg can open be opened by CAP_SYS_ADMIN */ > - if (type != 1 && from_file) > + if (type != SYSLOG_ACTION_OPEN && from_file) > return 0; > - if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN)) > + if ((type != SYSLOG_ACTION_READ_ALL && > + type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN)) > return -EPERM; > return 0; > } > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index a4862a0..6b36ce2 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -2059,20 +2059,21 @@ static int selinux_syslog(int type, bool from_file) > return rc; > > switch (type) { > - case 3: /* Read last kernel messages */ > - case 10: /* Return size of the log buffer */ > + case SYSLOG_ACTION_READ_ALL: /* Read last kernel messages */ > + case SYSLOG_ACTION_SIZE_BUFFER: /* Return size of the log buffer */ > rc = task_has_system(current, SYSTEM__SYSLOG_READ); > break; > - case 6: /* Disable logging to console */ > - case 7: /* Enable logging to console */ > - case 8: /* Set level of messages printed to console */ > + case SYSLOG_ACTION_CONSOLE_OFF: /* Disable logging to console */ > + case SYSLOG_ACTION_CONSOLE_ON: /* Enable logging to console */ > + /* Set level of messages printed to console */ > + case SYSLOG_ACTION_CONSOLE_LEVEL: > rc = task_has_system(current, SYSTEM__SYSLOG_CONSOLE); > break; > - case 0: /* Close log */ > - case 1: /* Open log */ > - case 2: /* Read from log */ > - case 4: /* Read/clear last kernel messages */ > - case 5: /* Clear ring buffer */ > + case SYSLOG_ACTION_CLOSE: /* Close log */ > + case SYSLOG_ACTION_OPEN: /* Open log */ > + case SYSLOG_ACTION_READ: /* Read from log */ > + case SYSLOG_ACTION_READ_CLEAR: /* Read/clear last kernel messages */ > + case SYSLOG_ACTION_CLEAR: /* Clear ring buffer */ > default: > rc = task_has_system(current, SYSTEM__SYSLOG_MOD); > break; > -- > 1.6.5 > > > -- > Kees Cook > Ubuntu Security Team -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/