public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 1/4] Add <linux/klog.h>
  2006-11-13  6:40 [patch 0/4] Syslog permissions, revised Zack Weinberg
@ 2006-11-13  6:40 ` Zack Weinberg
  0 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-11-13  6:40 UTC (permalink / raw)
  To: Chris Wright, Stephen Smalley, jmorris; +Cc: linux-kernel

[-- Attachment #1: add_klog_h.diff --]
[-- Type: text/plain, Size: 5846 bytes --]

This patch introduces <linux/klog.h> with symbolic constants for the
various sys_syslog() opcodes, and changes all in-kernel references to
those opcodes to use the constants.  The header is added to the set of
user/kernel interface headers; there is no #ifdef __KERNEL__ block in
it yet, but there will be in subsequent patches, so I put it in as an
'unifdef' header.

zw

Index: linux-2.6/include/linux/Kbuild
===================================================================
--- linux-2.6.orig/include/linux/Kbuild	2006-11-10 13:36:58.000000000 -0800
+++ linux-2.6/include/linux/Kbuild	2006-11-10 13:37:13.000000000 -0800
@@ -253,6 +253,7 @@
 unifdef-y += kernelcapi.h
 unifdef-y += kernel.h
 unifdef-y += keyboard.h
+unifdef-y += klog.h
 unifdef-y += llc.h
 unifdef-y += loop.h
 unifdef-y += lp.h
Index: linux-2.6/include/linux/klog.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/linux/klog.h	2006-11-10 14:16:35.000000000 -0800
@@ -0,0 +1,26 @@
+#ifndef _LINUX_KLOG_H
+#define _LINUX_KLOG_H
+
+/*
+ * Constants for the first argument to the syslog() system call
+ * (aka klogctl()).  These numbers are part of the user space ABI!
+ */
+enum {
+	KLOG_CLOSE           =  0, /* close log */
+	KLOG_OPEN            =  1, /* open log */
+	KLOG_READ            =  2, /* read from log (klogd) */
+
+	KLOG_READ_HIST       =  3, /* read history of log messages (dmesg) */
+	KLOG_READ_CLEAR_HIST =  4, /* read and clear history */
+	KLOG_CLEAR_HIST      =  5, /* just clear history */
+
+	KLOG_DISABLE_CONSOLE =  6, /* disable printk to console */
+	KLOG_ENABLE_CONSOLE  =  7, /* enable printk to console */
+	KLOG_SET_CONSOLE_LVL =  8, /* set minimum severity of messages to be
+				    * printed to console */
+
+	KLOG_GET_UNREAD      =  9, /* return number of unread characters */
+	KLOG_GET_SIZE        = 10  /* return size of log buffer */
+};
+
+#endif /* klog.h */
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c	2006-11-10 13:16:47.000000000 -0800
+++ linux-2.6/kernel/printk.c	2006-11-10 14:16:34.000000000 -0800
@@ -32,6 +32,7 @@
 #include <linux/bootmem.h>
 #include <linux/syscalls.h>
 #include <linux/jiffies.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 
@@ -165,21 +166,7 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
-/*
- * 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
- */
+/* See linux/klog.h for the command numbers passed as the first argument.  */
 int do_syslog(int type, char __user *buf, int len)
 {
 	unsigned long i, j, limit, count;
@@ -192,11 +179,11 @@
 		return error;
 
 	switch (type) {
-	case 0:		/* Close log */
+	case KLOG_CLOSE:
 		break;
-	case 1:		/* Open log */
+	case KLOG_OPEN:
 		break;
-	case 2:		/* Read from log */
+	case KLOG_READ:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -227,10 +214,10 @@
 		if (!error)
 			error = i;
 		break;
-	case 4:		/* Read/clear last kernel messages */
+	case KLOG_READ_CLEAR_HIST:
 		do_clear = 1;
 		/* FALL THRU */
-	case 3:		/* Read last kernel messages */
+	case KLOG_READ_HIST:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -283,16 +270,16 @@
 			}
 		}
 		break;
-	case 5:		/* Clear ring buffer */
+	case KLOG_CLEAR_HIST:
 		logged_chars = 0;
 		break;
-	case 6:		/* Disable logging to console */
+	case KLOG_DISABLE_CONSOLE:
 		console_loglevel = minimum_console_loglevel;
 		break;
-	case 7:		/* Enable logging to console */
+	case KLOG_ENABLE_CONSOLE:
 		console_loglevel = default_console_loglevel;
 		break;
-	case 8:		/* Set level of messages printed to console */
+	case KLOG_SET_CONSOLE_LVL:
 		error = -EINVAL;
 		if (len < 1 || len > 8)
 			goto out;
@@ -301,10 +288,10 @@
 		console_loglevel = len;
 		error = 0;
 		break;
-	case 9:		/* Number of chars in the log buffer */
+	case KLOG_GET_UNREAD:
 		error = log_end - log_start;
 		break;
-	case 10:	/* Size of the log buffer */
+	case KLOG_GET_SIZE:
 		error = log_buf_len;
 		break;
 	default:
Index: linux-2.6/fs/proc/kmsg.c
===================================================================
--- linux-2.6.orig/fs/proc/kmsg.c	2006-11-10 14:16:41.000000000 -0800
+++ linux-2.6/fs/proc/kmsg.c	2006-11-10 14:22:56.000000000 -0800
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/poll.h>
 #include <linux/fs.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -21,27 +22,28 @@
 
 static int kmsg_open(struct inode * inode, struct file * file)
 {
-	return do_syslog(1,NULL,0);
+	return do_syslog(KLOG_OPEN,NULL,0);
 }
 
 static int kmsg_release(struct inode * inode, struct file * file)
 {
-	(void) do_syslog(0,NULL,0);
+	(void) do_syslog(KLOG_CLOSE,NULL,0);
 	return 0;
 }
 
 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))
+	if ((file->f_flags & O_NONBLOCK)
+	    && !do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return -EAGAIN;
-	return do_syslog(2, buf, count);
+	return do_syslog(KLOG_READ, buf, count);
 }
 
 static unsigned int kmsg_poll(struct file *file, poll_table *wait)
 {
 	poll_wait(file, &log_wait, wait);
-	if (do_syslog(9, NULL, 0))
+	if (do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return POLLIN | POLLRDNORM;
 	return 0;
 }

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 1/4] Add <linux/klog.h>
  2006-12-15  0:16 [patch 0/4] /proc/kmsg permissions, take three Zack Weinberg
@ 2006-12-15  0:16 ` Zack Weinberg
  2006-12-15  0:59   ` Randy Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Zack Weinberg @ 2006-12-15  0:16 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright; +Cc: linux-kernel

[-- Attachment #1: add_klog_h.diff --]
[-- Type: text/plain, Size: 5835 bytes --]

This patch introduces <linux/klog.h> with symbolic constants for the
various sys_syslog() opcodes, and changes all in-kernel references to
those opcodes to use the constants.  The header is added to the set of
user/kernel interface headers.  (Unlike the previous revision of this
patch series, no kernel-private additions to this file are contemplated.)

zw

Index: linux-2.6/include/linux/Kbuild
===================================================================
--- linux-2.6.orig/include/linux/Kbuild	2006-12-13 15:58:13.000000000 -0800
+++ linux-2.6/include/linux/Kbuild	2006-12-13 16:06:57.000000000 -0800
@@ -100,6 +100,7 @@
 header-y += ixjuser.h
 header-y += jffs2.h
 header-y += keyctl.h
+header-y += klog.h
 header-y += limits.h
 header-y += lock_dlm_plock.h
 header-y += magic.h
Index: linux-2.6/include/linux/klog.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/linux/klog.h	2006-12-13 16:06:22.000000000 -0800
@@ -0,0 +1,26 @@
+#ifndef _LINUX_KLOG_H
+#define _LINUX_KLOG_H
+
+/*
+ * Constants for the first argument to the syslog() system call
+ * (aka klogctl()).  These numbers are part of the user space ABI!
+ */
+enum {
+	KLOG_CLOSE           =  0, /* close log */
+	KLOG_OPEN            =  1, /* open log */
+	KLOG_READ            =  2, /* read from log (klogd) */
+
+	KLOG_READ_HIST       =  3, /* read history of log messages (dmesg) */
+	KLOG_READ_CLEAR_HIST =  4, /* read and clear history */
+	KLOG_CLEAR_HIST      =  5, /* just clear history */
+
+	KLOG_DISABLE_CONSOLE =  6, /* disable printk to console */
+	KLOG_ENABLE_CONSOLE  =  7, /* enable printk to console */
+	KLOG_SET_CONSOLE_LVL =  8, /* set minimum severity of messages to be
+				    * printed to console */
+
+	KLOG_GET_UNREAD      =  9, /* return number of unread characters */
+	KLOG_GET_SIZE        = 10  /* return size of log buffer */
+};
+
+#endif /* klog.h */
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c	2006-12-13 15:58:16.000000000 -0800
+++ linux-2.6/kernel/printk.c	2006-12-13 16:06:22.000000000 -0800
@@ -32,6 +32,7 @@
 #include <linux/bootmem.h>
 #include <linux/syscalls.h>
 #include <linux/jiffies.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 
@@ -163,21 +164,7 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
-/*
- * 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
- */
+/* See linux/klog.h for the command numbers passed as the first argument.  */
 int do_syslog(int type, char __user *buf, int len)
 {
 	unsigned long i, j, limit, count;
@@ -190,11 +177,11 @@
 		return error;
 
 	switch (type) {
-	case 0:		/* Close log */
+	case KLOG_CLOSE:
 		break;
-	case 1:		/* Open log */
+	case KLOG_OPEN:
 		break;
-	case 2:		/* Read from log */
+	case KLOG_READ:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -225,10 +212,10 @@
 		if (!error)
 			error = i;
 		break;
-	case 4:		/* Read/clear last kernel messages */
+	case KLOG_READ_CLEAR_HIST:
 		do_clear = 1;
 		/* FALL THRU */
-	case 3:		/* Read last kernel messages */
+	case KLOG_READ_HIST:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -281,16 +268,16 @@
 			}
 		}
 		break;
-	case 5:		/* Clear ring buffer */
+	case KLOG_CLEAR_HIST:
 		logged_chars = 0;
 		break;
-	case 6:		/* Disable logging to console */
+	case KLOG_DISABLE_CONSOLE:
 		console_loglevel = minimum_console_loglevel;
 		break;
-	case 7:		/* Enable logging to console */
+	case KLOG_ENABLE_CONSOLE:
 		console_loglevel = default_console_loglevel;
 		break;
-	case 8:		/* Set level of messages printed to console */
+	case KLOG_SET_CONSOLE_LVL:
 		error = -EINVAL;
 		if (len < 1 || len > 8)
 			goto out;
@@ -299,10 +286,10 @@
 		console_loglevel = len;
 		error = 0;
 		break;
-	case 9:		/* Number of chars in the log buffer */
+	case KLOG_GET_UNREAD:
 		error = log_end - log_start;
 		break;
-	case 10:	/* Size of the log buffer */
+	case KLOG_GET_SIZE:
 		error = log_buf_len;
 		break;
 	default:
Index: linux-2.6/fs/proc/kmsg.c
===================================================================
--- linux-2.6.orig/fs/proc/kmsg.c	2006-12-13 15:53:29.000000000 -0800
+++ linux-2.6/fs/proc/kmsg.c	2006-12-13 16:04:46.000000000 -0800
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/poll.h>
 #include <linux/fs.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -21,27 +22,28 @@
 
 static int kmsg_open(struct inode * inode, struct file * file)
 {
-	return do_syslog(1,NULL,0);
+	return do_syslog(KLOG_OPEN,NULL,0);
 }
 
 static int kmsg_release(struct inode * inode, struct file * file)
 {
-	(void) do_syslog(0,NULL,0);
+	(void) do_syslog(KLOG_CLOSE,NULL,0);
 	return 0;
 }
 
 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))
+	if ((file->f_flags & O_NONBLOCK)
+	    && !do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return -EAGAIN;
-	return do_syslog(2, buf, count);
+	return do_syslog(KLOG_READ, buf, count);
 }
 
 static unsigned int kmsg_poll(struct file *file, poll_table *wait)
 {
 	poll_wait(file, &log_wait, wait);
-	if (do_syslog(9, NULL, 0))
+	if (do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return POLLIN | POLLRDNORM;
 	return 0;
 }

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 1/4] Add <linux/klog.h>
  2006-12-15  0:16 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
@ 2006-12-15  0:59   ` Randy Dunlap
  2006-12-15  1:21     ` Zack Weinberg
  0 siblings, 1 reply; 12+ messages in thread
From: Randy Dunlap @ 2006-12-15  0:59 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Stephen Smalley, jmorris, Chris Wright, linux-kernel

On Thu, 14 Dec 2006 16:16:40 -0800 Zack Weinberg wrote:

> This patch introduces <linux/klog.h> with symbolic constants for the
> various sys_syslog() opcodes, and changes all in-kernel references to
> those opcodes to use the constants.  The header is added to the set of
> user/kernel interface headers.  (Unlike the previous revision of this
> patch series, no kernel-private additions to this file are contemplated.)

Hi Zack,

This patch looks good except for one nit:

> --- linux-2.6.orig/fs/proc/kmsg.c	2006-12-13 15:53:29.000000000 -0800
> +++ linux-2.6/fs/proc/kmsg.c	2006-12-13 16:04:46.000000000 -0800
> @@ -21,27 +22,28 @@
>  
>  static int kmsg_open(struct inode * inode, struct file * file)
>  {
> -	return do_syslog(1,NULL,0);
> +	return do_syslog(KLOG_OPEN,NULL,0);
>  }
>  
>  static int kmsg_release(struct inode * inode, struct file * file)
>  {
> -	(void) do_syslog(0,NULL,0);
> +	(void) do_syslog(KLOG_CLOSE,NULL,0);
>  	return 0;
>  }

Please use a space after the commas (even though you just left it
as it already was).

---
~Randy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 1/4] Add <linux/klog.h>
  2006-12-15  0:59   ` Randy Dunlap
@ 2006-12-15  1:21     ` Zack Weinberg
  0 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-15  1:21 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Stephen Smalley, jmorris, Chris Wright, linux-kernel

On 12/14/06, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > -     (void) do_syslog(0,NULL,0);
> > +     (void) do_syslog(KLOG_CLOSE,NULL,0);
>
> Please use a space after the commas (even though you just left it
> as it already was).

Will change for the next revision.

zw

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 1/4] Add <linux/klog.h>
@ 2006-12-20  0:34 Vincent Legoll
  0 siblings, 0 replies; 12+ messages in thread
From: Vincent Legoll @ 2006-12-20  0:34 UTC (permalink / raw)
  To: zackw; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 181 bytes --]

Hello,

what about something along the lines of the following,
on top of your patch ?

Or should the kernel-doc be put on another function
instead of that one ?

-- 
Vincent Legoll

[-- Attachment #2: do-syslog-kernel-doc --]
[-- Type: text/plain, Size: 1178 bytes --]

Add do_syslog() kernel-doc

---
commit 95b0721d8b4b46ddf83113fe49492810d7d92060
tree e2715a8cf7eb0d71b3bee2185a5cf98639d79d90
parent de794d2dfd6dd0c38dd552020ac00c46e1df5293
author Vincent Legoll <vincent.legoll@gmail.com> Wed, 20 Dec 2006 01:29:34 +0100
committer Vincent Legoll <vincent.legoll@gmail.com> Wed, 20 Dec 2006 01:29:34 +0100

 kernel/printk.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 232467e..5416d07 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -164,7 +164,16 @@ out:
 
 __setup("log_buf_len=", log_buf_len_setup);
 
-/* See linux/klog.h for the command numbers passed as the first argument.  */
+/**
+ * do_syslog - operate on kernel messages log
+ * @type: operation to perform
+ * @buf: user-space buffer to copy data into
+ * @len: length of data to copy from log into @buf
+ *
+ * See include/linux/klog.h for the command numbers passed as @type.
+ * Parameters @buf & @len are only used for operations of type %KLOG_READ,
+ * %KLOG_READ_HIST and %KLOG_READ_CLEAR_HIST.
+ */
 int do_syslog(int type, char __user *buf, int len)
 {
 	unsigned long i, j, limit, count;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [patch 0/4] /proc/kmsg permissions, take four
@ 2006-12-24 20:22 Zack Weinberg
  2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-24 20:22 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll; +Cc: linux-kernel

Here's yet another revision of the /proc/kmsg permissions patch
series.  To recap, the point is to allow klogd to drop privileges
and continue reading from /proc/kmsg (currently, even if klogd has a
legitimately opened fd on /proc/kmsg, it cannot read from it unless
it has CAP_SYS_ADMIN asserted).  SELinux's pickier and finer-grained
privilege rules for /proc/kmsg are unchanged.

There are two significant changes from the previous revision.  First,
in keeping with the recommended style, I have eliminated the
security_syslog_or_fail() macro.  Instead there is a static array mapping
KLOG_* opcodes to LSM_KLOG_* privilege classes.  This requires slightly
different coding in the security hooks but I think it's clearer overall.
Second, I've incorporated Vincent Legoll's kerneldoc comment for sys_syslog
(nee do_syslog) with some wording improvements and expansion to cover the
klog_* functions introduced part-way through the patch.  I don't think
proc/kmsg.c needs kerneldoc, it's very simple after this patch series.

I've been through Documentation/CodingStyle and satisfied myself that
everything is now in the proper mode.  I don't suppose anyone has comments
on the *content* of the changes...?

zw


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 1/4] Add <linux/klog.h>
  2006-12-24 20:22 [patch 0/4] /proc/kmsg permissions, take four Zack Weinberg
@ 2006-12-24 20:22 ` Zack Weinberg
  2006-12-24 21:00   ` Jan Engelhardt
  2006-12-27 23:25   ` Vincent Legoll
  2006-12-24 20:22 ` [patch 2/4] permission mapping for sys_syslog operations Zack Weinberg
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-24 20:22 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll; +Cc: linux-kernel

[-- Attachment #1: add_klog_h.diff --]
[-- Type: text/plain, Size: 6322 bytes --]

This patch introduces <linux/klog.h> with symbolic constants for the
various sys_syslog() opcodes, and changes all in-kernel references to
those opcodes to use the constants.  The header is added to the set of
user/kernel interface headers.

zw

Index: linux-2.6/include/linux/Kbuild
===================================================================
--- linux-2.6.orig/include/linux/Kbuild	2006-12-23 08:56:15.000000000 -0800
+++ linux-2.6/include/linux/Kbuild	2006-12-24 11:43:14.000000000 -0800
@@ -100,6 +100,7 @@
 header-y += ixjuser.h
 header-y += jffs2.h
 header-y += keyctl.h
+header-y += klog.h
 header-y += limits.h
 header-y += lock_dlm_plock.h
 header-y += magic.h
Index: linux-2.6/include/linux/klog.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6/include/linux/klog.h	2006-12-24 11:43:14.000000000 -0800
@@ -0,0 +1,28 @@
+#ifndef _LINUX_KLOG_H
+#define _LINUX_KLOG_H
+
+/*
+ * Constants for the first argument to the syslog() system call
+ * (aka klogctl()).  These numbers are part of the user space ABI!
+ */
+enum {
+	KLOG_CLOSE           =  0, /* close log */
+	KLOG_OPEN            =  1, /* open log */
+	KLOG_READ            =  2, /* read from log (klogd) */
+
+	KLOG_READ_HIST       =  3, /* read history of log messages (dmesg) */
+	KLOG_READ_CLEAR_HIST =  4, /* read and clear history */
+	KLOG_CLEAR_HIST      =  5, /* just clear history */
+
+	KLOG_DISABLE_CONSOLE =  6, /* disable printk to console */
+	KLOG_ENABLE_CONSOLE  =  7, /* enable printk to console */
+	KLOG_SET_CONSOLE_LVL =  8, /* set minimum severity of messages to be
+				    * printed to console */
+
+	KLOG_GET_UNREAD      =  9, /* return number of unread characters */
+	KLOG_GET_SIZE        = 10, /* return size of log buffer */
+
+	KLOG_OPCODE_MAX
+};
+
+#endif /* klog.h */
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c	2006-12-23 08:56:15.000000000 -0800
+++ linux-2.6/kernel/printk.c	2006-12-24 11:43:14.000000000 -0800
@@ -32,6 +32,7 @@
 #include <linux/bootmem.h>
 #include <linux/syscalls.h>
 #include <linux/jiffies.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 
@@ -163,20 +164,21 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
-/*
- * Commands to do_syslog:
+/**
+ * do_syslog - operate on the log of kernel messages
+ * @type: operation to perform
+ * @buf: user-space buffer to copy data into
+ * @len: number of bytes of space available at @buf
+ *
+ * See include/linux/klog.h for the command numbers passed as @type.
+ * The @buf and @len parameters are used with the above meanings for
+ * @type values %KLOG_READ, %KLOG_READ_HIST and %KLOG_READ_CLEAR_HIST.
+ * @len is reused with a different meaning, and @buf ignored, for
+ * %KLOG_SET_CONSOLE_LVL.  Both @buf and @len are ignored for all
+ * other @type values.
  *
- * 	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
+ * On failure, returns a negative errno code.  On success, returns a
+ * nonnegative integer whose meaning depends on @type.
  */
 int do_syslog(int type, char __user *buf, int len)
 {
@@ -190,11 +192,11 @@
 		return error;
 
 	switch (type) {
-	case 0:		/* Close log */
+	case KLOG_CLOSE:
 		break;
-	case 1:		/* Open log */
+	case KLOG_OPEN:
 		break;
-	case 2:		/* Read from log */
+	case KLOG_READ:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -225,10 +227,10 @@
 		if (!error)
 			error = i;
 		break;
-	case 4:		/* Read/clear last kernel messages */
+	case KLOG_READ_CLEAR_HIST:
 		do_clear = 1;
 		/* FALL THRU */
-	case 3:		/* Read last kernel messages */
+	case KLOG_READ_HIST:
 		error = -EINVAL;
 		if (!buf || len < 0)
 			goto out;
@@ -281,16 +283,16 @@
 			}
 		}
 		break;
-	case 5:		/* Clear ring buffer */
+	case KLOG_CLEAR_HIST:
 		logged_chars = 0;
 		break;
-	case 6:		/* Disable logging to console */
+	case KLOG_DISABLE_CONSOLE:
 		console_loglevel = minimum_console_loglevel;
 		break;
-	case 7:		/* Enable logging to console */
+	case KLOG_ENABLE_CONSOLE:
 		console_loglevel = default_console_loglevel;
 		break;
-	case 8:		/* Set level of messages printed to console */
+	case KLOG_SET_CONSOLE_LVL:
 		error = -EINVAL;
 		if (len < 1 || len > 8)
 			goto out;
@@ -299,10 +301,10 @@
 		console_loglevel = len;
 		error = 0;
 		break;
-	case 9:		/* Number of chars in the log buffer */
+	case KLOG_GET_UNREAD:
 		error = log_end - log_start;
 		break;
-	case 10:	/* Size of the log buffer */
+	case KLOG_GET_SIZE:
 		error = log_buf_len;
 		break;
 	default:
Index: linux-2.6/fs/proc/kmsg.c
===================================================================
--- linux-2.6.orig/fs/proc/kmsg.c	2006-12-23 08:55:17.000000000 -0800
+++ linux-2.6/fs/proc/kmsg.c	2006-12-24 11:43:14.000000000 -0800
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/poll.h>
 #include <linux/fs.h>
+#include <linux/klog.h>
 
 #include <asm/uaccess.h>
 #include <asm/io.h>
@@ -21,27 +22,28 @@
 
 static int kmsg_open(struct inode * inode, struct file * file)
 {
-	return do_syslog(1,NULL,0);
+	return do_syslog(KLOG_OPEN, NULL, 0);
 }
 
 static int kmsg_release(struct inode * inode, struct file * file)
 {
-	(void) do_syslog(0,NULL,0);
+	(void) do_syslog(KLOG_CLOSE, NULL, 0);
 	return 0;
 }
 
 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))
+	if ((file->f_flags & O_NONBLOCK)
+	    && !do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return -EAGAIN;
-	return do_syslog(2, buf, count);
+	return do_syslog(KLOG_READ, buf, count);
 }
 
 static unsigned int kmsg_poll(struct file *file, poll_table *wait)
 {
 	poll_wait(file, &log_wait, wait);
-	if (do_syslog(9, NULL, 0))
+	if (do_syslog(KLOG_GET_UNREAD, NULL, 0))
 		return POLLIN | POLLRDNORM;
 	return 0;
 }

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 2/4] permission mapping for sys_syslog operations
  2006-12-24 20:22 [patch 0/4] /proc/kmsg permissions, take four Zack Weinberg
  2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
@ 2006-12-24 20:22 ` Zack Weinberg
  2006-12-24 20:22 ` [patch 3/4] Refactor do_syslog interface Zack Weinberg
  2006-12-24 20:22 ` [patch 4/4] Distinguish /proc/kmsg access from sys_syslog Zack Weinberg
  3 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-24 20:22 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll; +Cc: linux-kernel

[-- Attachment #1: map_perms.diff --]
[-- Type: text/plain, Size: 6597 bytes --]

As suggested by Stephen Smalley: map the various sys_syslog operations
to a smaller set of privilege codes before calling security modules.
This patch changes the security module interface!  There should be no
change in the actual security semantics enforced by dummy, capability,
nor SELinux (with one exception, clearly marked in sys_syslog).

The privilege codes are now in linux/security.h instead of
linux/klog.h, and use the LSM_* naming convention used for other such
constants in that file.

I'm now using a static table to map operations to privilege codes.
This makes it very easy to see the mapping as a whole, and prevents
security operations from cluttering up the body of do_syslog, but does
necessitate some checks to ensure that when new KLOG_* operations are
added, people don't forget to add entries to the table.  If people are
not fans of the idiom I used, I will see about getting a feature added
to gcc to make it less ugly.

Note that after this patch, close() on /proc/kmsg and
sys_syslog(KLOG_CLOSE) do not do security checks; they always succeed.
(IMO close should never fail.) This was always in this patch series,
but used to be in a different stage.

zw

Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c	2006-12-24 11:43:14.000000000 -0800
+++ linux-2.6/kernel/printk.c	2006-12-24 11:43:16.000000000 -0800
@@ -164,6 +164,39 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
+/*
+ * This table maps KLOG_* operation codes to LSM_KLOG_* privilege classes.
+ * "unsigned char" is used just to keep it small.
+ */
+
+static const unsigned char klog_privclass[] = {
+	[KLOG_CLOSE]           = 0,  /* close always succeeds */
+	[KLOG_OPEN]            = LSM_KLOG_READ,
+	[KLOG_READ]            = LSM_KLOG_READ,
+	[KLOG_GET_UNREAD]      = LSM_KLOG_READ,
+
+	[KLOG_READ_HIST]       = LSM_KLOG_READHIST,
+	[KLOG_GET_SIZE]        = LSM_KLOG_READHIST,
+	[KLOG_READ_CLEAR_HIST] = LSM_KLOG_READHIST|LSM_KLOG_CLEARHIST,
+	[KLOG_CLEAR_HIST]      = LSM_KLOG_CLEARHIST,
+
+	[KLOG_DISABLE_CONSOLE] = LSM_KLOG_CONSOLE,
+	[KLOG_ENABLE_CONSOLE]  = LSM_KLOG_CONSOLE,
+	[KLOG_SET_CONSOLE_LVL] = LSM_KLOG_CONSOLE,
+};
+
+/* It is essential that there be an entry in the above table for every
+ * KLOG_* code.  The following, er, odd declarations cause compilation
+ * of this file to fail if that is not true.  They do not correspond
+ * to real data objects.
+ */
+extern char klog_privclass_is_missing_an_entry[
+	ARRAY_SIZE(klog_privclass) < KLOG_OPCODE_MAX ? -1 : 1
+];
+extern char klog_privclass_has_too_many_entries[
+	ARRAY_SIZE(klog_privclass) > KLOG_OPCODE_MAX ? -1 : 1
+];
+
 /**
  * do_syslog - operate on the log of kernel messages
  * @type: operation to perform
@@ -185,9 +218,11 @@
 	unsigned long i, j, limit, count;
 	int do_clear = 0;
 	char c;
-	int error = 0;
+	int error;
 
-	error = security_syslog(type);
+	if (type < 0 || type >= KLOG_OPCODE_MAX)
+		return -EINVAL;
+	error = security_syslog(klog_privclass[type]);
 	if (error)
 		return error;
 
Index: linux-2.6/security/commoncap.c
===================================================================
--- linux-2.6.orig/security/commoncap.c	2006-12-23 08:55:16.000000000 -0800
+++ linux-2.6/security/commoncap.c	2006-12-24 11:43:16.000000000 -0800
@@ -311,7 +311,7 @@
 
 int cap_syslog (int type)
 {
-	if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN))
+	if ((type & ~LSM_KLOG_READHIST) && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	return 0;
 }
Index: linux-2.6/security/dummy.c
===================================================================
--- linux-2.6.orig/security/dummy.c	2006-12-23 08:55:16.000000000 -0800
+++ linux-2.6/security/dummy.c	2006-12-24 11:43:16.000000000 -0800
@@ -96,7 +96,7 @@
 
 static int dummy_syslog (int type)
 {
-	if ((type != 3 && type != 10) && current->euid)
+	if ((type & ~LSM_KLOG_READHIST) && current->euid)
 		return -EPERM;
 	return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===================================================================
--- linux-2.6.orig/security/selinux/hooks.c	2006-12-23 08:55:16.000000000 -0800
+++ linux-2.6/security/selinux/hooks.c	2006-12-24 11:43:16.000000000 -0800
@@ -1504,29 +1504,27 @@
 {
 	int rc;
 
+	/* if there are any codes we don't know about, there's a bug */
+	BUG_ON(type & ~(LSM_KLOG_READ|LSM_KLOG_READHIST
+			|LSM_KLOG_CLEARHIST|LSM_KLOG_CONSOLE));
+
 	rc = secondary_ops->syslog(type);
 	if (rc)
 		return rc;
 
-	switch (type) {
-		case 3:         /* Read last kernel messages */
-		case 10:        /* 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 */
-			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 */
-		default:
-			rc = task_has_system(current, SYSTEM__SYSLOG_MOD);
-			break;
-	}
+	if (type & (LSM_KLOG_READ|LSM_KLOG_CLEARHIST))
+		rc = task_has_system(current, SYSTEM__SYSLOG_MOD);
+	if (rc)
+		return rc;
+
+	if (type & LSM_KLOG_READHIST)
+		rc = task_has_system(current, SYSTEM__SYSLOG_READ);
+	if (rc)
+		return rc;
+
+	if (type & LSM_KLOG_CONSOLE)
+		rc = task_has_system(current, SYSTEM__SYSLOG_CONSOLE);
+
 	return rc;
 }
 
Index: linux-2.6/include/linux/security.h
===================================================================
--- linux-2.6.orig/include/linux/security.h	2006-12-23 08:55:16.000000000 -0800
+++ linux-2.6/include/linux/security.h	2006-12-24 11:43:16.000000000 -0800
@@ -86,6 +86,18 @@
 /* setfsuid or setfsgid, id0 == fsuid or fsgid */
 #define LSM_SETID_FS	8
 
+/*
+ * Values passed to security_syslog to indicate which privilege class
+ * is requested.  requested.  Note these are bitmasks - callers may
+ * request more than one of the privilege classes at once (this is
+ * currently only used for READHIST|CLEARHIST) or no privilege at all
+ * (which should always succeed).
+ */
+#define LSM_KLOG_READ      1  /* read current messages (klogd) */
+#define LSM_KLOG_READHIST  2  /* read message history (dmesg) */
+#define LSM_KLOG_CLEARHIST 4  /* clear message history (dmesg -c) */
+#define LSM_KLOG_CONSOLE   8  /* set or query console log level */
+
 /* forward declares to avoid warnings */
 struct nfsctl_arg;
 struct sched_param;

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 3/4] Refactor do_syslog interface
  2006-12-24 20:22 [patch 0/4] /proc/kmsg permissions, take four Zack Weinberg
  2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
  2006-12-24 20:22 ` [patch 2/4] permission mapping for sys_syslog operations Zack Weinberg
@ 2006-12-24 20:22 ` Zack Weinberg
  2006-12-24 20:22 ` [patch 4/4] Distinguish /proc/kmsg access from sys_syslog Zack Weinberg
  3 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-24 20:22 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll; +Cc: linux-kernel

[-- Attachment #1: break_up_do_syslog.diff --]
[-- Type: text/plain, Size: 12951 bytes --]

This patch breaks out the read operations in do_syslog() into their
own functions (klog_read, klog_readhist) and adds a klog_poll.
klog_read grows the ability to do a nonblocking read, which I expose
in the sys_syslog interface because there doesn't seem to be any
reason not to.  do_syslog itself is folded into sys_syslog.  The
security checks remain there, not in the subfunctions.  I've added
kerneldoc comments for all the new functions in printk.c.  The control
flow in sys_syslog is different in style but not in substance -- I
like direct returns rather than setting a local variable and returning
at the end; it lets me put a BUG() after the switch statement to catch
missed cases.  [gcc4 at least can optimize it out when there aren't
any.]

kmsg.c is then changed to use those functions instead of calling
do_syslog and/or poll_wait itself.. This entails that it must call
security_syslog as appropriate itself.  In this patch I preserve the
security checks exactly as they were after the changes in the "map
permissions" patch (i.e. kmsg_close() doesn't do a permission check).

Finally, I fixed some minor bugs. Error and partial read handling in
klog_read/klog_readhist were slightly off: if __put_user returns an
error, that character should not be consumed from the kernel buffer;
if it returns an error after some characters have already been copied
successfully, the read operation should return the count of
already-copied characters, not the error code; if less than the entire
buffer is read with KLOG_READ_CLEAR_HIST, we should only clear the
part that was successfully read.  Seeking on /proc/kmsg has never been
meaningful, so kmsg_open() should call nonseekable_open() to enforce
that.

proc/kmsg.c declares the kernel/printk.c interfaces itself, instead of
getting them from klog.h which people want to be purely userspace-
visible constants.  kmsg.c has always had private declarations of
printk.c functions (before, there were declarations of do_syslog and a
wait queue there); as it is unlikely that more users of these
functions will appear, I think this will do fine.  (It might be
reasonable to put declarations in console.h.)

It was possible to prune down the set of headers included by kmsg.c
quite substantially.

zw

Index: linux-2.6/fs/proc/kmsg.c
===================================================================
--- linux-2.6.orig/fs/proc/kmsg.c	2006-12-24 11:43:14.000000000 -0800
+++ linux-2.6/fs/proc/kmsg.c	2006-12-24 11:43:18.000000000 -0800
@@ -5,47 +5,42 @@
  *
  */
 
-#include <linux/types.h>
-#include <linux/errno.h>
-#include <linux/time.h>
-#include <linux/kernel.h>
-#include <linux/poll.h>
 #include <linux/fs.h>
-#include <linux/klog.h>
-
-#include <asm/uaccess.h>
-#include <asm/io.h>
-
-extern wait_queue_head_t log_wait;
+#include <linux/poll.h>
+#include <linux/security.h>
 
-extern int do_syslog(int type, char __user *bug, int count);
+/* interfaces from kernel/printk.c */
+extern int klog_read(char __user *, int, int);
+extern unsigned int klog_poll(struct file *, poll_table *);
 
 static int kmsg_open(struct inode * inode, struct file * file)
 {
-	return do_syslog(KLOG_OPEN, NULL, 0);
+	int error = security_syslog(LSM_KLOG_READ);
+	if (error)
+		return error;
+	return nonseekable_open(inode, file);
 }
 
 static int kmsg_release(struct inode * inode, struct file * file)
 {
-	(void) do_syslog(KLOG_CLOSE, NULL, 0);
 	return 0;
 }
 
 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(KLOG_GET_UNREAD, NULL, 0))
-		return -EAGAIN;
-	return do_syslog(KLOG_READ, buf, count);
+	int error = security_syslog(LSM_KLOG_READ);
+	if (error)
+		return error;
+	return klog_read(buf, count, !(file->f_flags & O_NONBLOCK));
 }
 
 static unsigned int kmsg_poll(struct file *file, poll_table *wait)
 {
-	poll_wait(file, &log_wait, wait);
-	if (do_syslog(KLOG_GET_UNREAD, NULL, 0))
-		return POLLIN | POLLRDNORM;
-	return 0;
+	int error = security_syslog(LSM_KLOG_READ);
+	if (error)
+		return error;
+	return klog_poll(file, wait);
 }
 
 
Index: linux-2.6/include/linux/klog.h
===================================================================
--- linux-2.6.orig/include/linux/klog.h	2006-12-24 11:43:14.000000000 -0800
+++ linux-2.6/include/linux/klog.h	2006-12-24 11:43:18.000000000 -0800
@@ -21,6 +21,8 @@
 
 	KLOG_GET_UNREAD      =  9, /* return number of unread characters */
 	KLOG_GET_SIZE        = 10, /* return size of log buffer */
+ 	KLOG_READ_NONBLOCK   = 11, /* read from log, don't block if empty
+ 				    * -- new in 2.6.20 */
 
 	KLOG_OPCODE_MAX
 };
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c	2006-12-24 11:43:16.000000000 -0800
+++ linux-2.6/kernel/printk.c	2006-12-24 11:43:18.000000000 -0800
@@ -33,6 +33,7 @@
 #include <linux/syscalls.h>
 #include <linux/jiffies.h>
 #include <linux/klog.h>
+#include <linux/poll.h>
 
 #include <asm/uaccess.h>
 
@@ -45,7 +46,7 @@
 #define MINIMUM_CONSOLE_LOGLEVEL 1 /* Minimum loglevel we let people use */
 #define DEFAULT_CONSOLE_LOGLEVEL 7 /* anything MORE serious than KERN_DEBUG */
 
-DECLARE_WAIT_QUEUE_HEAD(log_wait);
+static DECLARE_WAIT_QUEUE_HEAD(log_wait);
 
 int console_printk[4] = {
 	DEFAULT_CONSOLE_LOGLEVEL,	/* console_loglevel */
@@ -165,6 +166,136 @@
 __setup("log_buf_len=", log_buf_len_setup);
 
 /*
+ * Subfunctions of sys_syslog.  Some are also used from fs/proc/kmsg.c.
+ */
+
+/**
+ * klog_read - read current messages from the kernel log buffer.
+ * @buf: user-space buffer to copy data into
+ * @len: number of bytes of space available at @buf
+ * @block: if nonzero, block until data is available.
+ *
+ * Return value is as for the read() system call.
+ */
+int klog_read(char __user *buf, int len, int block)
+{
+	int i, error;
+	char c;
+
+	if (!buf || len < 0)
+		return -EINVAL;
+	if (len == 0)
+		return 0;
+	if (!access_ok(VERIFY_WRITE, buf, len))
+		return -EFAULT;
+	if (!block && log_start - log_end == 0)
+		return -EAGAIN;
+
+	error = wait_event_interruptible(log_wait, (log_start - log_end));
+	if (error)
+		return error;
+
+	i = 0;
+	spin_lock_irq(&logbuf_lock);
+	while (log_start != log_end && i < len) {
+		c = LOG_BUF(log_start);
+		spin_unlock_irq(&logbuf_lock);
+		error = __put_user(c,buf);
+		if (error)
+			goto out;
+		cond_resched();
+		spin_lock_irq(&logbuf_lock);
+		log_start++;
+		buf++;
+		i++;
+	}
+	spin_unlock_irq(&logbuf_lock);
+out:
+	if (i > 0)
+		return i;
+	return error;
+}
+
+/**
+ * klog_poll - poll() worker for the kernel log buffer.
+ */
+unsigned int klog_poll(struct file *file, poll_table *wait)
+{
+	poll_wait(file, &log_wait, wait);
+	if (log_end - log_start > 0)
+		return POLLIN | POLLRDNORM;
+	return 0;
+}
+
+/**
+ * klog_read - read historic messages from the kernel log buffer.
+ * @buf: user-space buffer to copy data into
+ * @len: number of bytes of space available at @buf
+ * @block: if nonzero, block until data is available.
+ *
+ * Return value is as for the read() system call.  The difference
+ * between this and klog_read() is that this function will copy out all
+ * the messages that have ever been reported by the kernel, as long as
+ * they've not been overwritten by newer messages; whereas klog_read()
+ * only ever copies out a given message once.
+ */
+static int klog_readhist(char __user *buf, int len)
+{
+	unsigned long i, j, limit, count;
+	char c;
+	int error = 0;
+
+	if (!buf || len < 0)
+		return -EINVAL;
+	if (!len)
+		return 0;
+	if (!access_ok(VERIFY_WRITE, buf, len))
+		return -EFAULT;
+
+	count = len;
+	if (count > log_buf_len)
+		count = log_buf_len;
+	spin_lock_irq(&logbuf_lock);
+	if (count > logged_chars)
+		count = logged_chars;
+	limit = log_end;
+	/*
+	 * __put_user() could sleep, and while we sleep
+	 * printk() could overwrite the messages
+	 * we try to copy to user space. Therefore
+	 * the messages are copied in reverse. <manfreds>
+	 */
+	for (i = 0; i < count && !error; i++) {
+		j = limit-1-i;
+		if (j + log_buf_len < log_end)
+			break;
+		c = LOG_BUF(j);
+		spin_unlock_irq(&logbuf_lock);
+		error = __put_user(c,&buf[count-1-i]);
+		cond_resched();
+		spin_lock_irq(&logbuf_lock);
+	}
+	spin_unlock_irq(&logbuf_lock);
+	if (error)
+		return error;
+	error = i;
+	if (i != count) {
+		int offset = count-error;
+		/* buffer overflow during copy, correct user buffer. */
+		for (i = 0; i < error; i++) {
+			if (__get_user(c,&buf[i+offset]) ||
+			    __put_user(c,&buf[i])) {
+				error = -EFAULT;
+				break;
+			}
+			cond_resched();
+		}
+	}
+	return error;
+}
+
+
+/*
  * This table maps KLOG_* operation codes to LSM_KLOG_* privilege classes.
  * "unsigned char" is used just to keep it small.
  */
@@ -174,6 +305,7 @@
 	[KLOG_OPEN]            = LSM_KLOG_READ,
 	[KLOG_READ]            = LSM_KLOG_READ,
 	[KLOG_GET_UNREAD]      = LSM_KLOG_READ,
+	[KLOG_READ_NONBLOCK]   = LSM_KLOG_READ,
 
 	[KLOG_READ_HIST]       = LSM_KLOG_READHIST,
 	[KLOG_GET_SIZE]        = LSM_KLOG_READHIST,
@@ -198,7 +330,7 @@
 ];
 
 /**
- * do_syslog - operate on the log of kernel messages
+ * sys_syslog - operate on the log of kernel messages
  * @type: operation to perform
  * @buf: user-space buffer to copy data into
  * @len: number of bytes of space available at @buf
@@ -213,11 +345,8 @@
  * On failure, returns a negative errno code.  On success, returns a
  * nonnegative integer whose meaning depends on @type.
  */
-int do_syslog(int type, char __user *buf, int len)
+asmlinkage long sys_syslog(int type, char __user *buf, int len)
 {
-	unsigned long i, j, limit, count;
-	int do_clear = 0;
-	char c;
 	int error;
 
 	if (type < 0 || type >= KLOG_OPCODE_MAX)
@@ -228,131 +357,51 @@
 
 	switch (type) {
 	case KLOG_CLOSE:
-		break;
 	case KLOG_OPEN:
-		break;
+		return 0;
+
 	case KLOG_READ:
-		error = -EINVAL;
-		if (!buf || len < 0)
-			goto out;
-		error = 0;
-		if (!len)
-			goto out;
-		if (!access_ok(VERIFY_WRITE, buf, len)) {
-			error = -EFAULT;
-			goto out;
-		}
-		error = wait_event_interruptible(log_wait,
-							(log_start - log_end));
-		if (error)
-			goto out;
-		i = 0;
-		spin_lock_irq(&logbuf_lock);
-		while (!error && (log_start != log_end) && i < len) {
-			c = LOG_BUF(log_start);
-			log_start++;
-			spin_unlock_irq(&logbuf_lock);
-			error = __put_user(c,buf);
-			buf++;
-			i++;
-			cond_resched();
-			spin_lock_irq(&logbuf_lock);
-		}
-		spin_unlock_irq(&logbuf_lock);
-		if (!error)
-			error = i;
-		break;
+	case KLOG_READ_NONBLOCK:
+		return klog_read(buf, len, type == KLOG_READ);
+
 	case KLOG_READ_CLEAR_HIST:
-		do_clear = 1;
-		/* FALL THRU */
+		error = klog_readhist(buf, len);
+		if (error > 0)
+			logged_chars -= error;
+		return error;
+
 	case KLOG_READ_HIST:
-		error = -EINVAL;
-		if (!buf || len < 0)
-			goto out;
-		error = 0;
-		if (!len)
-			goto out;
-		if (!access_ok(VERIFY_WRITE, buf, len)) {
-			error = -EFAULT;
-			goto out;
-		}
-		count = len;
-		if (count > log_buf_len)
-			count = log_buf_len;
-		spin_lock_irq(&logbuf_lock);
-		if (count > logged_chars)
-			count = logged_chars;
-		if (do_clear)
-			logged_chars = 0;
-		limit = log_end;
-		/*
-		 * __put_user() could sleep, and while we sleep
-		 * printk() could overwrite the messages
-		 * we try to copy to user space. Therefore
-		 * the messages are copied in reverse. <manfreds>
-		 */
-		for (i = 0; i < count && !error; i++) {
-			j = limit-1-i;
-			if (j + log_buf_len < log_end)
-				break;
-			c = LOG_BUF(j);
-			spin_unlock_irq(&logbuf_lock);
-			error = __put_user(c,&buf[count-1-i]);
-			cond_resched();
-			spin_lock_irq(&logbuf_lock);
-		}
-		spin_unlock_irq(&logbuf_lock);
-		if (error)
-			break;
-		error = i;
-		if (i != count) {
-			int offset = count-error;
-			/* buffer overflow during copy, correct user buffer. */
-			for (i = 0; i < error; i++) {
-				if (__get_user(c,&buf[i+offset]) ||
-				    __put_user(c,&buf[i])) {
-					error = -EFAULT;
-					break;
-				}
-				cond_resched();
-			}
-		}
-		break;
+		return klog_readhist(buf, len);
+
 	case KLOG_CLEAR_HIST:
 		logged_chars = 0;
-		break;
+		return 0;
+
 	case KLOG_DISABLE_CONSOLE:
 		console_loglevel = minimum_console_loglevel;
-		break;
+		return 0;
+
 	case KLOG_ENABLE_CONSOLE:
 		console_loglevel = default_console_loglevel;
-		break;
+		return 0;
+
 	case KLOG_SET_CONSOLE_LVL:
-		error = -EINVAL;
 		if (len < 1 || len > 8)
-			goto out;
+			return -EINVAL;
+
 		if (len < minimum_console_loglevel)
 			len = minimum_console_loglevel;
 		console_loglevel = len;
-		error = 0;
-		break;
+		return 0;
+
 	case KLOG_GET_UNREAD:
-		error = log_end - log_start;
-		break;
+		return log_end - log_start;
+
 	case KLOG_GET_SIZE:
-		error = log_buf_len;
-		break;
-	default:
-		error = -EINVAL;
-		break;
+		return log_buf_len;
 	}
-out:
-	return error;
-}
 
-asmlinkage long sys_syslog(int type, char __user *buf, int len)
-{
-	return do_syslog(type, buf, len);
+	BUG();
 }
 
 /*

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 4/4] Distinguish /proc/kmsg access from sys_syslog
  2006-12-24 20:22 [patch 0/4] /proc/kmsg permissions, take four Zack Weinberg
                   ` (2 preceding siblings ...)
  2006-12-24 20:22 ` [patch 3/4] Refactor do_syslog interface Zack Weinberg
@ 2006-12-24 20:22 ` Zack Weinberg
  3 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2006-12-24 20:22 UTC (permalink / raw)
  To: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll; +Cc: linux-kernel

[-- Attachment #1: distinguish_kmsg_security.diff --]
[-- Type: text/plain, Size: 4869 bytes --]

Finally, add a new security class for access to /proc/kmsg, distinct
from the class used for the "read current messages" operations on
sys_syslog.  The dummy and capability modules permit access to
/proc/kmsg to any user (who has somehow acquired an open fd on it);
SELinux is unchanged.  This accomplishes what I was trying to do in
the first place, i.e. enable running klogd unprivileged without a root
shim, in a non-SELinux installation.  Please remember that the
default DAC permissions for /proc/kmsg restrict it to root, so unless
you chmod it in your installation or modify klogd to open the file and
then drop privs, the actual restrictions are unchanged.

zw


Index: linux-2.6/fs/proc/kmsg.c
===================================================================
--- linux-2.6.orig/fs/proc/kmsg.c	2006-12-24 11:43:18.000000000 -0800
+++ linux-2.6/fs/proc/kmsg.c	2006-12-24 11:43:19.000000000 -0800
@@ -15,7 +15,7 @@
 
 static int kmsg_open(struct inode * inode, struct file * file)
 {
-	int error = security_syslog(LSM_KLOG_READ);
+	int error = security_syslog(LSM_KLOG_READ_PROC);
 	if (error)
 		return error;
 	return nonseekable_open(inode, file);
@@ -29,7 +29,7 @@
 static ssize_t kmsg_read(struct file *file, char __user *buf,
 			 size_t count, loff_t *ppos)
 {
-	int error = security_syslog(LSM_KLOG_READ);
+	int error = security_syslog(LSM_KLOG_READ_PROC);
 	if (error)
 		return error;
 	return klog_read(buf, count, !(file->f_flags & O_NONBLOCK));
@@ -37,7 +37,7 @@
 
 static unsigned int kmsg_poll(struct file *file, poll_table *wait)
 {
-	int error = security_syslog(LSM_KLOG_READ);
+	int error = security_syslog(LSM_KLOG_READ_PROC);
 	if (error)
 		return error;
 	return klog_poll(file, wait);
Index: linux-2.6/security/commoncap.c
===================================================================
--- linux-2.6.orig/security/commoncap.c	2006-12-24 11:43:16.000000000 -0800
+++ linux-2.6/security/commoncap.c	2006-12-24 11:43:19.000000000 -0800
@@ -311,7 +311,14 @@
 
 int cap_syslog (int type)
 {
-	if ((type & ~LSM_KLOG_READHIST) && !capable(CAP_SYS_ADMIN))
+ 	/*
+ 	 * Reading history is allowed to any user, and so is reading
+ 	 * current messages via /proc/kmsg (by default that file is
+ 	 * only readable by root, but root is allowed to change that,
+ 	 * or open it and hand the fd to an unprivileged process).
+ 	 */
+	if ((type & ~(LSM_KLOG_READHIST|LSM_KLOG_READ_PROC))
+	    && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===================================================================
--- linux-2.6.orig/security/selinux/hooks.c	2006-12-24 11:43:16.000000000 -0800
+++ linux-2.6/security/selinux/hooks.c	2006-12-24 11:43:19.000000000 -0800
@@ -1505,14 +1505,20 @@
 	int rc;
 
 	/* if there are any codes we don't know about, there's a bug */
-	BUG_ON(type & ~(LSM_KLOG_READ|LSM_KLOG_READHIST
+	BUG_ON(type & ~(LSM_KLOG_READ|LSM_KLOG_READ_PROC|LSM_KLOG_READHIST
 			|LSM_KLOG_CLEARHIST|LSM_KLOG_CONSOLE));
 
 	rc = secondary_ops->syslog(type);
 	if (rc)
 		return rc;
 
-	if (type & (LSM_KLOG_READ|LSM_KLOG_CLEARHIST))
+	/*
+	 * N.B. Unlike the default security model, with
+	 * SELinux active you have to have SYSTEM__SYSLOG_MOD
+	 * privilege to read current messages either with the
+	 * system call or from /proc/kmsg.
+	 */
+	if (type & (LSM_KLOG_READ|LSM_KLOG_READ_PROC|LSM_KLOG_CLEARHIST))
 		rc = task_has_system(current, SYSTEM__SYSLOG_MOD);
 	if (rc)
 		return rc;
Index: linux-2.6/include/linux/security.h
===================================================================
--- linux-2.6.orig/include/linux/security.h	2006-12-24 11:43:16.000000000 -0800
+++ linux-2.6/include/linux/security.h	2006-12-24 11:43:19.000000000 -0800
@@ -97,6 +97,8 @@
 #define LSM_KLOG_READHIST  2  /* read message history (dmesg) */
 #define LSM_KLOG_CLEARHIST 4  /* clear message history (dmesg -c) */
 #define LSM_KLOG_CONSOLE   8  /* set or query console log level */
+#define LSM_KLOG_READ_PROC 16 /* read current messages, but from /proc/kmsg
+				 rather than the system call */
 
 /* forward declares to avoid warnings */
 struct nfsctl_arg;
Index: linux-2.6/security/dummy.c
===================================================================
--- linux-2.6.orig/security/dummy.c	2006-12-24 11:43:16.000000000 -0800
+++ linux-2.6/security/dummy.c	2006-12-24 11:43:19.000000000 -0800
@@ -96,7 +96,13 @@
 
 static int dummy_syslog (int type)
 {
-	if ((type & ~LSM_KLOG_READHIST) && current->euid)
+ 	/*
+ 	 * Reading history is allowed to any user, and so is reading
+ 	 * current messages via /proc/kmsg (by default that file is
+ 	 * only readable by root, but root is allowed to change that,
+ 	 * or open it and hand the fd to an unprivileged process).
+ 	 */
+	if ((type & ~(LSM_KLOG_READHIST|LSM_KLOG_READ_PROC)) && current->euid)
 		return -EPERM;
 	return 0;
 }

--


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 1/4] Add <linux/klog.h>
  2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
@ 2006-12-24 21:00   ` Jan Engelhardt
  2006-12-27 23:25   ` Vincent Legoll
  1 sibling, 0 replies; 12+ messages in thread
From: Jan Engelhardt @ 2006-12-24 21:00 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Stephen Smalley, jmorris, Chris Wright, Vincent Legoll,
	linux-kernel


On Dec 24 2006 12:22, Zack Weinberg wrote:
>===================================================================
>--- linux-2.6.orig/fs/proc/kmsg.c	2006-12-23 08:55:17.000000000 -0800
>+++ linux-2.6/fs/proc/kmsg.c	2006-12-24 11:43:14.000000000 -0800
>@@ -11,6 +11,7 @@
> #include <linux/kernel.h>
> #include <linux/poll.h>
> #include <linux/fs.h>
>+#include <linux/klog.h>
> 
> #include <asm/uaccess.h>
> #include <asm/io.h>
>@@ -21,27 +22,28 @@
> 
> static int kmsg_open(struct inode * inode, struct file * file)
> {
>-	return do_syslog(1,NULL,0);
>+	return do_syslog(KLOG_OPEN, NULL, 0);
> }
> 
> static int kmsg_release(struct inode * inode, struct file * file)
> {
>-	(void) do_syslog(0,NULL,0);
>+	(void) do_syslog(KLOG_CLOSE, NULL, 0);
        ^^^^^^

I bet you can get rid of that in the process.


	-`J'
-- 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 1/4] Add <linux/klog.h>
  2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
  2006-12-24 21:00   ` Jan Engelhardt
@ 2006-12-27 23:25   ` Vincent Legoll
  1 sibling, 0 replies; 12+ messages in thread
From: Vincent Legoll @ 2006-12-27 23:25 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Stephen Smalley, jmorris, Chris Wright, linux-kernel

Zack Weinberg wrote:
> This patch introduces <linux/klog.h> with symbolic constants for the
> various sys_syslog() opcodes, and changes all in-kernel references to
> those opcodes to use the constants.  The header is added to the set of
> user/kernel interface headers.

[...]

> -/*
> - * Commands to do_syslog:
> +/**
> + * do_syslog - operate on the log of kernel messages
> + * @type: operation to perform
> + * @buf: user-space buffer to copy data into
> + * @len: number of bytes of space available at @buf
> + *
> + * See include/linux/klog.h for the command numbers passed as @type.
> + * The @buf and @len parameters are used with the above meanings for
> + * @type values %KLOG_READ, %KLOG_READ_HIST and %KLOG_READ_CLEAR_HIST.
> + * @len is reused with a different meaning, and @buf ignored, for
> + * %KLOG_SET_CONSOLE_LVL.  Both @buf and @len are ignored for all
> + * other @type values.
>   *
> - * 	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
> + * On failure, returns a negative errno code.  On success, returns a
> + * nonnegative integer whose meaning depends on @type.
>   */
>  int do_syslog(int type, char __user *buf, int len)
>  {
> @@ -190,11 +192,11 @@

That part looks good to me, and the kernel-doc hunks from
"[patch 3/4] Refactor do_syslog interface"
too, are:

Acked-by: Vincent Legoll <vincent.legoll@gmail.com>

thanks for handling that.

-- 
Vincent Legoll

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-12-27 23:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-24 20:22 [patch 0/4] /proc/kmsg permissions, take four Zack Weinberg
2006-12-24 20:22 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
2006-12-24 21:00   ` Jan Engelhardt
2006-12-27 23:25   ` Vincent Legoll
2006-12-24 20:22 ` [patch 2/4] permission mapping for sys_syslog operations Zack Weinberg
2006-12-24 20:22 ` [patch 3/4] Refactor do_syslog interface Zack Weinberg
2006-12-24 20:22 ` [patch 4/4] Distinguish /proc/kmsg access from sys_syslog Zack Weinberg
  -- strict thread matches above, loose matches on Subject: below --
2006-12-20  0:34 [patch 1/4] Add <linux/klog.h> Vincent Legoll
2006-12-15  0:16 [patch 0/4] /proc/kmsg permissions, take three Zack Weinberg
2006-12-15  0:16 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg
2006-12-15  0:59   ` Randy Dunlap
2006-12-15  1:21     ` Zack Weinberg
2006-11-13  6:40 [patch 0/4] Syslog permissions, revised Zack Weinberg
2006-11-13  6:40 ` [patch 1/4] Add <linux/klog.h> Zack Weinberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox