[PATCH 1/1] Syslog are now containerized

[PATCH 1/1] Syslog are now containerized

[Jean-Marc Pigeon]

	Added syslog.c such container /proc/kmsg and host /proc/kmsg
	do not leak in each other.
	Running rsyslog daemon within a container won't destroy
	host kernel messages.
---
 Makefile                       |    2 +-
 include/linux/syslog.h         |   29 ++++
 include/linux/user_namespace.h |    1 +
 kernel/Makefile                |    2 +-
 kernel/printk.c                |  292 ++++++++++++++++++----------------------
 kernel/syslog.c                |  157 +++++++++++++++++++++
 kernel/user.c                  |    6 +-
 kernel/user_namespace.c        |    9 ++
 8 files changed, 334 insertions(+), 164 deletions(-)
 create mode 100644 include/linux/syslog.h
 create mode 100644 kernel/syslog.c

diff --git a/Makefile b/Makefile
index f8e02e9..4fec8dd 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 VERSION = 2
 PATCHLEVEL = 6
 SUBLEVEL = 33
-EXTRAVERSION = -rc7
+EXTRAVERSION = -rc7		
 NAME = Man-Eating Seals of Antiquity
 
 # *DOCUMENTATION*
diff --git a/include/linux/syslog.h b/include/linux/syslog.h
new file mode 100644
index 0000000..98c6898
--- /dev/null
+++ b/include/linux/syslog.h
@@ -0,0 +1,29 @@
+#ifndef _LINUX_SYSLOG_H
+#define _LINUX_SYSLOG_H
+#include	<linux/spinlock_types.h>
+
+struct syslog_ns {
+        wait_queue_head_t wait;
+	spinlock_t logbuf_lock;	/* access conflict locker				*/
+        unsigned log_start;	/* Index into log_buf: next char to be read by syslog() */
+        unsigned con_start;	/* Index into log_buf: next char to be sent to consoles */
+        unsigned log_end;	/* Index into log_buf: most-recently-written-char + 1	*/
+        unsigned logged_chars;	/* Number of chars produced since last read+clear access*/
+        unsigned buf_len;	/* buffer available space size				*/
+        char *buf;		/* allocated ring buffer				*/
+};
+
+/*
+ *  Static structure used by kernel
+ */
+extern struct syslog_ns init_kernel_syslog_ns;
+
+/*
+ * Syslog API
+ *
+ */
+extern struct syslog_ns *syslog_malloc(unsigned container_buf_len);
+extern struct syslog_ns *syslog_realloc(struct syslog_ns *syslog_ns, unsigned container_buf_len);
+extern struct syslog_ns *syslog_free(struct syslog_ns *syslog);
+extern struct syslog_ns *syslog_get_current(void);
+#endif /* _LINUX_SYSLOG_H */
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index cc4f453..3d0c73e 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -14,6 +14,7 @@ struct user_namespace {
 	struct hlist_head	uidhash_table[UIDHASH_SZ];
 	struct user_struct	*creator;
 	struct work_struct	destroyer;
+	struct syslog_ns        *syslog;
 };
 
 extern struct user_namespace init_user_ns;
diff --git a/kernel/Makefile b/kernel/Makefile
index 864ff75..e20d64e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the linux kernel.
 #
 
-obj-y     = sched.o fork.o exec_domain.o panic.o printk.o \
+obj-y     = sched.o fork.o exec_domain.o panic.o printk.o syslog.o \
 	    cpu.o exit.o itimer.o time.o softirq.o resource.o \
 	    sysctl.o sysctl_binary.o capability.o ptrace.o timer.o user.o \
 	    signal.o sys.o kmod.o workqueue.o pid.o \
diff --git a/kernel/printk.c b/kernel/printk.c
index 1751c45..fd0a05c 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -35,6 +35,7 @@
 #include <linux/kexec.h>
 #include <linux/ratelimit.h>
 #include <linux/kmsg_dump.h>
+#include <linux/syslog.h>
 
 #include <asm/uaccess.h>
 
@@ -51,8 +52,6 @@ void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
 {
 }
 
-#define __LOG_BUF_LEN	(1 << CONFIG_LOG_BUF_SHIFT)
-
 /* printk's without a loglevel use this.. */
 #define DEFAULT_MESSAGE_LOGLEVEL 4 /* KERN_WARNING */
 
@@ -69,8 +68,6 @@ int console_printk[4] = {
 	DEFAULT_CONSOLE_LOGLEVEL,	/* default_console_loglevel */
 };
 
-static int saved_console_loglevel = -1;
-
 /*
  * Low level drivers may need that to know if they can schedule in
  * their unblank() callback or not. So let's export it.
@@ -97,23 +94,20 @@ EXPORT_SYMBOL_GPL(console_drivers);
  */
 static int console_locked, console_suspended;
 
-/*
- * logbuf_lock protects log_buf, log_start, log_end, con_start and logged_chars
- * It is also used in interesting ways to provide interlocking in
- * release_console_sem().
- */
-static DEFINE_SPINLOCK(logbuf_lock);
-
-#define LOG_BUF_MASK (log_buf_len-1)
-#define LOG_BUF(idx) (log_buf[(idx) & LOG_BUF_MASK])
+#define LOG_BUF_MASK(ns) ((ns)->buf_len-1)
+#define LOG_BUF(ns, idx) ((ns)->buf[(idx) & LOG_BUF_MASK(ns)])
 
 /*
- * The indices into log_buf are not constrained to log_buf_len - they
- * must be masked before subscripting
+ * To access container syslog ring buffer
  */
-static unsigned log_start;	/* Index into log_buf: next char to be read by syslog() */
-static unsigned con_start;	/* Index into log_buf: next char to be sent to consoles */
-static unsigned log_end;	/* Index into log_buf: most-recently-written-char + 1 */
+#define sys_log_wait (syslog_ns->wait)
+#define sys_log_lock (syslog_ns->logbuf_lock)
+#define sys_log_start (syslog_ns->log_start)
+#define sys_log_end (syslog_ns->log_end)
+#define sys_log_con_start (syslog_ns->con_start)
+#define sys_log_buf_len (syslog_ns->buf_len)
+#define sys_log_logged_chars (syslog_ns->logged_chars)
+#define sys_log_buf (syslog_ns->buf)
 
 /*
  *	Array of consoles built from command line options (console=)
@@ -141,10 +135,7 @@ static int console_may_schedule;
 
 #ifdef CONFIG_PRINTK
 
-static char __log_buf[__LOG_BUF_LEN];
-static char *log_buf = __log_buf;
-static int log_buf_len = __LOG_BUF_LEN;
-static unsigned logged_chars; /* Number of chars produced since last read+clear operation */
+static int saved_console_loglevel = -1;
 
 #ifdef CONFIG_KEXEC
 /*
@@ -157,49 +148,23 @@ static unsigned logged_chars; /* Number of chars produced since last read+clear
  */
 void log_buf_kexec_setup(void)
 {
-	VMCOREINFO_SYMBOL(log_buf);
-	VMCOREINFO_SYMBOL(log_end);
-	VMCOREINFO_SYMBOL(log_buf_len);
-	VMCOREINFO_SYMBOL(logged_chars);
+	struct syslog_ns *syslog_ns = syslog_get_current();
+
+	VMCOREINFO_SYMBOL(sys_log_buf);
+	VMCOREINFO_SYMBOL(sys_log_end);
+	VMCOREINFO_SYMBOL(sys_log_buf_len);
+	VMCOREINFO_SYMBOL(sys_log_logged_chars);
 }
 #endif
 
 static int __init log_buf_len_setup(char *str)
 {
 	unsigned size = memparse(str, &str);
-	unsigned long flags;
 
-	if (size)
+	if (size) {
 		size = roundup_pow_of_two(size);
-	if (size > log_buf_len) {
-		unsigned start, dest_idx, offset;
-		char *new_log_buf;
-
-		new_log_buf = alloc_bootmem(size);
-		if (!new_log_buf) {
-			printk(KERN_WARNING "log_buf_len: allocation failed\n");
-			goto out;
-		}
-
-		spin_lock_irqsave(&logbuf_lock, flags);
-		log_buf_len = size;
-		log_buf = new_log_buf;
-
-		offset = start = min(con_start, log_start);
-		dest_idx = 0;
-		while (start != log_end) {
-			log_buf[dest_idx] = __log_buf[start & (__LOG_BUF_LEN - 1)];
-			start++;
-			dest_idx++;
-		}
-		log_start -= offset;
-		con_start -= offset;
-		log_end -= offset;
-		spin_unlock_irqrestore(&logbuf_lock, flags);
-
-		printk(KERN_NOTICE "log_buf_len: %d\n", log_buf_len);
+		(void) syslog_realloc(&init_kernel_syslog_ns,size);
 	}
-out:
 	return 1;
 }
 
@@ -279,6 +244,7 @@ int do_syslog(int type, char __user *buf, int len)
 	int do_clear = 0;
 	char c;
 	int error = 0;
+	struct syslog_ns *syslog_ns = syslog_get_current();
 
 	error = security_syslog(type);
 	if (error)
@@ -300,23 +266,22 @@ int do_syslog(int type, char __user *buf, int len)
 			error = -EFAULT;
 			goto out;
 		}
-		error = wait_event_interruptible(log_wait,
-							(log_start - log_end));
+		error = wait_event_interruptible(sys_log_wait, (sys_log_start - sys_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);
+		spin_lock_irq(&sys_log_lock);
+		while (!error && (sys_log_start != sys_log_end) && i < len) {
+			c = LOG_BUF(syslog_ns, sys_log_start);
+			sys_log_start++;
+			spin_unlock_irq(&sys_log_lock);
 			error = __put_user(c,buf);
 			buf++;
 			i++;
 			cond_resched();
-			spin_lock_irq(&logbuf_lock);
+			spin_lock_irq(&sys_log_lock);
 		}
-		spin_unlock_irq(&logbuf_lock);
+		spin_unlock_irq(&sys_log_lock);
 		if (!error)
 			error = i;
 		break;
@@ -335,14 +300,14 @@ int do_syslog(int type, char __user *buf, int len)
 			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 (count > sys_log_buf_len)
+			count = sys_log_buf_len;
+		spin_lock_irq(&sys_log_lock);
+		if (count > sys_log_logged_chars)
+			count = sys_log_logged_chars;
 		if (do_clear)
-			logged_chars = 0;
-		limit = log_end;
+			sys_log_logged_chars = 0;
+		limit = sys_log_end;
 		/*
 		 * __put_user() could sleep, and while we sleep
 		 * printk() could overwrite the messages
@@ -351,15 +316,15 @@ int do_syslog(int type, char __user *buf, int len)
 		 */
 		for (i = 0; i < count && !error; i++) {
 			j = limit-1-i;
-			if (j + log_buf_len < log_end)
+			if (j + sys_log_buf_len < sys_log_end)
 				break;
-			c = LOG_BUF(j);
-			spin_unlock_irq(&logbuf_lock);
+			c = LOG_BUF(syslog_ns, j);
+			spin_unlock_irq(&sys_log_lock);
 			error = __put_user(c,&buf[count-1-i]);
 			cond_resched();
-			spin_lock_irq(&logbuf_lock);
+			spin_lock_irq(&sys_log_lock);
 		}
-		spin_unlock_irq(&logbuf_lock);
+		spin_unlock_irq(&sys_log_lock);
 		if (error)
 			break;
 		error = i;
@@ -377,7 +342,7 @@ int do_syslog(int type, char __user *buf, int len)
 		}
 		break;
 	case 5:		/* Clear ring buffer */
-		logged_chars = 0;
+		sys_log_logged_chars = 0;
 		break;
 	case 6:		/* Disable logging to console */
 		if (saved_console_loglevel == -1)
@@ -402,10 +367,10 @@ int do_syslog(int type, char __user *buf, int len)
 		error = 0;
 		break;
 	case 9:		/* Number of chars in the log buffer */
-		error = log_end - log_start;
+		error = sys_log_end - sys_log_start;
 		break;
 	case 10:	/* Size of the log buffer */
-		error = log_buf_len;
+		error = sys_log_buf_len;
 		break;
 	default:
 		error = -EINVAL;
@@ -423,7 +388,7 @@ SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
 /*
  * Call the console drivers on a range of log_buf
  */
-static void __call_console_drivers(unsigned start, unsigned end)
+static void __call_console_drivers(struct syslog_ns *syslog_ns,unsigned start, unsigned end)
 {
 	struct console *con;
 
@@ -431,7 +396,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
 		if ((con->flags & CON_ENABLED) && con->write &&
 				(cpu_online(smp_processor_id()) ||
 				(con->flags & CON_ANYTIME)))
-			con->write(con, &LOG_BUF(start), end - start);
+			con->write(con, &LOG_BUF(syslog_ns, start), end - start);
 	}
 }
 
@@ -450,18 +415,18 @@ early_param("ignore_loglevel", ignore_loglevel_setup);
 /*
  * Write out chars from start to end - 1 inclusive
  */
-static void _call_console_drivers(unsigned start,
-				unsigned end, int msg_log_level)
+static void _call_console_drivers(struct syslog_ns *syslog_ns, unsigned start,
+				  unsigned end, int msg_log_level)
 {
 	if ((msg_log_level < console_loglevel || ignore_loglevel) &&
 			console_drivers && start != end) {
-		if ((start & LOG_BUF_MASK) > (end & LOG_BUF_MASK)) {
+		if ((start & LOG_BUF_MASK(syslog_ns)) > (end & LOG_BUF_MASK(syslog_ns))) {
 			/* wrapped write */
-			__call_console_drivers(start & LOG_BUF_MASK,
-						log_buf_len);
-			__call_console_drivers(0, end & LOG_BUF_MASK);
+			__call_console_drivers(syslog_ns, start & LOG_BUF_MASK(syslog_ns),
+						sys_log_buf_len);
+			__call_console_drivers(syslog_ns,0, end & LOG_BUF_MASK(syslog_ns));
 		} else {
-			__call_console_drivers(start, end);
+			__call_console_drivers(syslog_ns, start, end);
 		}
 	}
 }
@@ -471,7 +436,7 @@ static void _call_console_drivers(unsigned start,
  * log_buf[start] to log_buf[end - 1].
  * The console_sem must be held.
  */
-static void call_console_drivers(unsigned start, unsigned end)
+static void call_console_drivers(struct syslog_ns *syslog_ns, unsigned start, unsigned end)
 {
 	unsigned cur_index, start_print;
 	static int msg_level = -1;
@@ -482,16 +447,16 @@ static void call_console_drivers(unsigned start, unsigned end)
 	start_print = start;
 	while (cur_index != end) {
 		if (msg_level < 0 && ((end - cur_index) > 2) &&
-				LOG_BUF(cur_index + 0) == '<' &&
-				LOG_BUF(cur_index + 1) >= '0' &&
-				LOG_BUF(cur_index + 1) <= '7' &&
-				LOG_BUF(cur_index + 2) == '>') {
-			msg_level = LOG_BUF(cur_index + 1) - '0';
+				LOG_BUF(syslog_ns, cur_index + 0) == '<' &&
+				LOG_BUF(syslog_ns, cur_index + 1) >= '0' &&
+				LOG_BUF(syslog_ns, cur_index + 1) <= '7' &&
+				LOG_BUF(syslog_ns, cur_index + 2) == '>') {
+			msg_level = LOG_BUF(syslog_ns, cur_index + 1) - '0';
 			cur_index += 3;
 			start_print = cur_index;
 		}
 		while (cur_index != end) {
-			char c = LOG_BUF(cur_index);
+			char c = LOG_BUF(syslog_ns, cur_index);
 
 			cur_index++;
 			if (c == '\n') {
@@ -504,26 +469,26 @@ static void call_console_drivers(unsigned start, unsigned end)
 					 */
 					msg_level = default_message_loglevel;
 				}
-				_call_console_drivers(start_print, cur_index, msg_level);
+				_call_console_drivers(syslog_ns, start_print, cur_index, msg_level);
 				msg_level = -1;
 				start_print = cur_index;
 				break;
 			}
 		}
 	}
-	_call_console_drivers(start_print, end, msg_level);
+	_call_console_drivers(syslog_ns, start_print, end, msg_level);
 }
 
-static void emit_log_char(char c)
+static void emit_log_char(struct syslog_ns *syslog_ns, char c)
 {
-	LOG_BUF(log_end) = c;
-	log_end++;
-	if (log_end - log_start > log_buf_len)
-		log_start = log_end - log_buf_len;
-	if (log_end - con_start > log_buf_len)
-		con_start = log_end - log_buf_len;
-	if (logged_chars < log_buf_len)
-		logged_chars++;
+	LOG_BUF(syslog_ns, sys_log_end) = c;
+	sys_log_end++;
+	if (sys_log_end - sys_log_start > sys_log_buf_len)
+		sys_log_start = sys_log_end - sys_log_buf_len;
+	if (sys_log_end - sys_log_con_start > sys_log_buf_len)
+		sys_log_con_start = sys_log_end - sys_log_buf_len;
+	if (sys_log_logged_chars < sys_log_buf_len)
+		sys_log_logged_chars++;
 }
 
 /*
@@ -531,7 +496,7 @@ static void emit_log_char(char c)
  * every 10 seconds, to leave time for slow consoles to print a
  * full oops.
  */
-static void zap_locks(void)
+static void zap_locks(struct syslog_ns *syslog_ns)
 {
 	static unsigned long oops_timestamp;
 
@@ -542,7 +507,7 @@ static void zap_locks(void)
 	oops_timestamp = jiffies;
 
 	/* If a crash is occurring, make sure we can't deadlock */
-	spin_lock_init(&logbuf_lock);
+	spin_lock_init(&sys_log_lock);
 	/* And make sure that we print immediately */
 	init_MUTEX(&console_sem);
 }
@@ -626,7 +591,7 @@ static inline int can_use_console(unsigned int cpu)
  * interrupts disabled. It should return with 'lockbuf_lock'
  * released but interrupts still disabled.
  */
-static int acquire_console_semaphore_for_printk(unsigned int cpu)
+static int acquire_console_semaphore_for_printk(struct syslog_ns *syslog_ns, unsigned int cpu)
 {
 	int retval = 0;
 
@@ -646,7 +611,7 @@ static int acquire_console_semaphore_for_printk(unsigned int cpu)
 		}
 	}
 	printk_cpu = UINT_MAX;
-	spin_unlock(&logbuf_lock);
+	spin_unlock(&sys_log_lock);
 	return retval;
 }
 static const char recursion_bug_msg [] =
@@ -673,6 +638,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 {
 	int printed_len = 0;
 	int current_log_level = default_message_loglevel;
+	struct syslog_ns *syslog_ns = syslog_get_current();
 	unsigned long flags;
 	int this_cpu;
 	char *p;
@@ -700,11 +666,11 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 			recursion_bug = 1;
 			goto out_restore_irqs;
 		}
-		zap_locks();
+		zap_locks(syslog_ns);
 	}
 
 	lockdep_off();
-	spin_lock(&logbuf_lock);
+	spin_lock(&sys_log_lock);
 	printk_cpu = this_cpu;
 
 	if (recursion_bug) {
@@ -729,7 +695,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 			/* Fallthrough - make sure we're on a new line */
 			case 'd': /* KERN_DEFAULT */
 				if (!new_text_line) {
-					emit_log_char('\n');
+					emit_log_char(syslog_ns, '\n');
 					new_text_line = 1;
 				}
 			/* Fallthrough - skip the loglevel */
@@ -747,9 +713,9 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 	for ( ; *p; p++) {
 		if (new_text_line) {
 			/* Always output the token */
-			emit_log_char('<');
-			emit_log_char(current_log_level + '0');
-			emit_log_char('>');
+			emit_log_char(syslog_ns, '<');
+			emit_log_char(syslog_ns, current_log_level + '0');
+			emit_log_char(syslog_ns, '>');
 			printed_len += 3;
 			new_text_line = 0;
 
@@ -767,7 +733,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 						nanosec_rem / 1000);
 
 				for (tp = tbuf; tp < tbuf + tlen; tp++)
-					emit_log_char(*tp);
+					emit_log_char(syslog_ns, *tp);
 				printed_len += tlen;
 			}
 
@@ -775,7 +741,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 				break;
 		}
 
-		emit_log_char(*p);
+		emit_log_char(syslog_ns, *p);
 		if (*p == '\n')
 			new_text_line = 1;
 	}
@@ -790,7 +756,7 @@ asmlinkage int vprintk(const char *fmt, va_list args)
 	 * will release 'logbuf_lock' regardless of whether it
 	 * actually gets the semaphore or not.
 	 */
-	if (acquire_console_semaphore_for_printk(this_cpu))
+	if (acquire_console_semaphore_for_printk(syslog_ns, this_cpu))
 		release_console_sem();
 
 	lockdep_on();
@@ -803,12 +769,6 @@ out_restore_irqs:
 EXPORT_SYMBOL(printk);
 EXPORT_SYMBOL(vprintk);
 
-#else
-
-static void call_console_drivers(unsigned start, unsigned end)
-{
-}
-
 #endif
 
 static int __add_preferred_console(char *name, int idx, char *options,
@@ -1041,36 +1001,40 @@ void wake_up_klogd(void)
  */
 void release_console_sem(void)
 {
-	unsigned long flags;
-	unsigned _con_start, _log_end;
-	unsigned wake_klogd = 0;
-
 	if (console_suspended) {
 		up(&console_sem);
 		return;
 	}
 
 	console_may_schedule = 0;
-
-	for ( ; ; ) {
-		spin_lock_irqsave(&logbuf_lock, flags);
-		wake_klogd |= log_start - log_end;
-		if (con_start == log_end)
-			break;			/* Nothing to print */
-		_con_start = con_start;
-		_log_end = log_end;
-		con_start = log_end;		/* Flush */
-		spin_unlock(&logbuf_lock);
-		stop_critical_timings();	/* don't trace print latency */
-		call_console_drivers(_con_start, _log_end);
-		start_critical_timings();
-		local_irq_restore(flags);
+#ifdef	CONFIG_PRINTK
+	{
+		unsigned long flags;
+		unsigned _con_start, _log_end;
+		unsigned wake_klogd = 0;
+		struct syslog_ns *syslog_ns = syslog_get_current();
+
+		for ( ; ; ) {
+			spin_lock_irqsave(&sys_log_lock, flags);
+			wake_klogd |= sys_log_start - sys_log_end;
+			if (sys_log_con_start == sys_log_end)
+				break;			/* Nothing to print */
+			_con_start = sys_log_con_start;
+			_log_end = sys_log_end;
+			sys_log_con_start = sys_log_end;	/* Flush */
+			spin_unlock(&sys_log_lock);
+			stop_critical_timings();	/* don't trace print latency */
+			call_console_drivers(syslog_ns, _con_start, _log_end);
+			start_critical_timings();
+			local_irq_restore(flags);
+		}
+		spin_unlock_irqrestore(&sys_log_lock, flags);
+		if (wake_klogd)
+			wake_up_klogd();
 	}
+#endif
 	console_locked = 0;
 	up(&console_sem);
-	spin_unlock_irqrestore(&logbuf_lock, flags);
-	if (wake_klogd)
-		wake_up_klogd();
 }
 EXPORT_SYMBOL(release_console_sem);
 
@@ -1175,7 +1139,6 @@ EXPORT_SYMBOL(console_start);
 void register_console(struct console *newcon)
 {
 	int i;
-	unsigned long flags;
 	struct console *bcon = NULL;
 
 	/*
@@ -1281,15 +1244,21 @@ void register_console(struct console *newcon)
 		newcon->next = console_drivers->next;
 		console_drivers->next = newcon;
 	}
+#ifdef	CONFIG_PRINTK
 	if (newcon->flags & CON_PRINTBUFFER) {
+		unsigned long flags;
 		/*
 		 * release_console_sem() will print out the buffered messages
 		 * for us.
 		 */
-		spin_lock_irqsave(&logbuf_lock, flags);
-		con_start = log_start;
-		spin_unlock_irqrestore(&logbuf_lock, flags);
+
+		struct syslog_ns *syslog_ns = syslog_get_current();
+
+		spin_lock_irqsave(&sys_log_lock, flags);
+		sys_log_con_start = sys_log_start;
+		spin_unlock_irqrestore(&sys_log_lock, flags);
 	}
+#endif
 	release_console_sem();
 
 	/*
@@ -1493,27 +1462,28 @@ void kmsg_dump(enum kmsg_dump_reason reason)
 	const char *s1, *s2;
 	unsigned long l1, l2;
 	unsigned long flags;
+	struct syslog_ns *syslog_ns = syslog_get_current();
 
 	/* Theoretically, the log could move on after we do this, but
 	   there's not a lot we can do about that. The new messages
 	   will overwrite the start of what we dump. */
-	spin_lock_irqsave(&logbuf_lock, flags);
-	end = log_end & LOG_BUF_MASK;
-	chars = logged_chars;
-	spin_unlock_irqrestore(&logbuf_lock, flags);
+	spin_lock_irqsave(&sys_log_lock, flags);
+	end = sys_log_end & LOG_BUF_MASK(syslog_ns);
+	chars = sys_log_logged_chars;
+	spin_unlock_irqrestore(&sys_log_lock, flags);
 
-	if (logged_chars > end) {
-		s1 = log_buf + log_buf_len - logged_chars + end;
-		l1 = logged_chars - end;
+	if (sys_log_logged_chars > end) {
+		s1 = sys_log_buf + sys_log_buf_len - sys_log_logged_chars + end;
+		l1 = sys_log_logged_chars - end;
 
-		s2 = log_buf;
+		s2 = sys_log_buf;
 		l2 = end;
 	} else {
 		s1 = "";
 		l1 = 0;
 
-		s2 = log_buf + end - logged_chars;
-		l2 = logged_chars;
+		s2 = sys_log_buf + end - sys_log_logged_chars;
+		l2 = sys_log_logged_chars;
 	}
 
 	if (!spin_trylock_irqsave(&dump_list_lock, flags)) {
diff --git a/kernel/syslog.c b/kernel/syslog.c
new file mode 100644
index 0000000..69d30a9
--- /dev/null
+++ b/kernel/syslog.c
@@ -0,0 +1,157 @@
+/*
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License as
+ *  published by the Free Software Foundation, version 2 of the
+ *  License.
+ *
+ *  Feb 2010
+ *  Serge E. Hallyn	<serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *  Jean-Marc Pigeon	<jmp-4qkeo2rQ0gg@public.gmane.org>
+ *
+ *  Purpose is to regroup all procedure involved
+ *  in system log.
+ *  System log need to be containerized to avoid
+ *  crossing over critical data between physical host layer
+ *  and container layer.
+ *
+ *  The principle is to keep a containerized ring buffer
+ *  where container kernel data are redirected, kept and
+ *  managed.
+ *
+ *  Containerized syslog is activated by CLONE_NEWUSER
+ *
+ */
+
+#include <linux/bootmem.h>
+#include <linux/slab.h>
+#include <linux/cred.h>
+#include <linux/user_namespace.h>
+#include <linux/syslog.h>
+
+/*
+ * Static memory definition, used to assign a syslog
+ * to the kernel itself
+ *
+ */
+
+#ifdef CONFIG_PRINTK
+#define __LOG_BUF_LEN   (1 << CONFIG_LOG_BUF_SHIFT)
+
+static char __log_buf[__LOG_BUF_LEN];
+
+struct syslog_ns init_kernel_syslog_ns = {
+        .wait = __WAIT_QUEUE_HEAD_INITIALIZER(init_kernel_syslog_ns.wait),
+        .buf_len = __LOG_BUF_LEN,
+        .buf = __log_buf
+};
+#endif
+
+/*
+ * Procedure to assign memory for syslog area
+ *
+ */
+struct syslog_ns * syslog_malloc(unsigned container_buf_len)
+{
+	struct syslog_ns *ns;
+
+	if (container_buf_len <= 0 )
+		return ERR_PTR(-EINVAL);
+
+	ns = kzalloc(sizeof(*ns), GFP_KERNEL);
+	if (!ns)
+		return ERR_PTR(-ENOMEM);
+
+	ns->buf_len = container_buf_len;
+	ns->buf = kzalloc(container_buf_len, GFP_KERNEL);
+	if (!ns->buf) {
+		(void) kfree(ns);
+		return ERR_PTR(-ENOMEM);
+	}
+	spin_lock_init(&(ns->logbuf_lock));
+	init_waitqueue_head(&ns->wait);
+	return ns;
+}
+
+/*
+ * Procedure to ONLY increase syslog buffer size
+ * If syslog_ns is NULL, assign a brand new syslog_ns
+ *
+ */
+struct syslog_ns * syslog_realloc(struct syslog_ns *syslog_ns, unsigned container_buf_len)
+
+{
+	if ((syslog_ns == &init_kernel_syslog_ns ) && (container_buf_len > syslog_ns->buf_len)) {
+		int old_buf_len;
+		char *old_buf;
+		char *new_buf;
+		unsigned long flags;
+
+		old_buf_len = syslog_ns->buf_len;
+		old_buf = syslog_ns->buf;
+		new_buf = alloc_bootmem(container_buf_len);
+		if (!new_buf) {
+			(void) printk(KERN_WARNING "log_buf_len: allocation failed\n");
+			return syslog_ns;
+			}
+		spin_lock_irqsave(&(syslog_ns->logbuf_lock), flags);
+		(void) memmove(new_buf, old_buf, old_buf_len);
+		syslog_ns->buf=new_buf;
+		syslog_ns->buf_len = container_buf_len;
+		spin_unlock_irqrestore(&(syslog_ns->logbuf_lock), flags);
+  		if (old_buf != __log_buf) 
+			(void) free_bootmem((unsigned long)old_buf, old_buf_len);
+		}
+	if (!syslog_ns)
+		return syslog_malloc(container_buf_len);
+	if (syslog_ns->buf_len > container_buf_len) {
+		(void) printk(KERN_WARNING "log_buf_len: Not allowed to decrease syslog buffer\n");
+		return ERR_PTR(-EINVAL);
+		}
+	if (syslog_ns->buf_len < container_buf_len) {
+		char *old_buf;
+		char *new_buf;
+		unsigned long flags;
+	
+		old_buf=syslog_ns->buf;	
+		new_buf=kzalloc(container_buf_len, GFP_KERNEL);
+		if (!new_buf)
+			return ERR_PTR(-ENOMEM);
+		spin_lock_irqsave(&(syslog_ns->logbuf_lock), flags);
+		(void) memmove(new_buf, old_buf, syslog_ns->buf_len);
+		syslog_ns->buf = new_buf;
+		syslog_ns->buf_len = container_buf_len;
+		spin_unlock_irqrestore(&(syslog_ns->logbuf_lock), flags);
+		(void) kfree(old_buf);
+		}
+	(void) printk(KERN_NOTICE "log_buf_len: %u\n", syslog_ns->buf_len);
+	return syslog_ns;
+}
+/*
+ * Procedure to free all ressources tied to syslog
+ *
+ */
+struct syslog_ns *syslog_free(struct syslog_ns *syslog)
+
+{
+	if (syslog != (struct syslog_ns *)0) {
+		(void) kfree(syslog->buf);
+		(void) kfree(syslog);
+		syslog = (struct syslog_ns *)0;
+		}
+	return syslog;
+}
+
+/*
+ * Procedure to get the current syslog area linked to a container (by CLONE_USER)
+ * if trouble, pin down the problem before it propagate.
+ *
+ */
+struct syslog_ns *syslog_get_current(void) 
+
+{
+	struct syslog_ns *ns;
+
+	ns = current_user_ns()->syslog;
+	BUG_ON(!ns);
+	return ns;
+}
diff --git a/kernel/user.c b/kernel/user.c
index 46d0165..cb2d4ba 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -16,13 +16,17 @@
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/user_namespace.h>
+#include <linux/syslog.h>
 #include "cred-internals.h"
 
 struct user_namespace init_user_ns = {
 	.kref = {
 		.refcount	= ATOMIC_INIT(2),
 	},
-	.creator = &root_user,
+#ifdef CONFIG_PRINTK
+	.syslog = &init_kernel_syslog_ns,
+#endif
+	.creator = &root_user
 };
 EXPORT_SYMBOL_GPL(init_user_ns);
 
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 076c7c8..9d8014f 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -10,6 +10,7 @@
 #include <linux/slab.h>
 #include <linux/user_namespace.h>
 #include <linux/cred.h>
+#include <linux/syslog.h>
 
 /*
  * Create a new user namespace, deriving the creator from the user in the
@@ -21,6 +22,8 @@
  */
 int create_user_ns(struct cred *new)
 {
+#define	CONTAINER_BUF_LEN	4096	/*should be enough for container syslog	*/
+
 	struct user_namespace *ns;
 	struct user_struct *root_user;
 	int n;
@@ -34,6 +37,12 @@ int create_user_ns(struct cred *new)
 	for (n = 0; n < UIDHASH_SZ; ++n)
 		INIT_HLIST_HEAD(ns->uidhash_table + n);
 
+	
+	ns->syslog = syslog_malloc(CONTAINER_BUF_LEN);
+        if (!ns->syslog) {
+                kfree(ns);
+                return -ENOMEM;
+        }
 	/* Alloc new root user.  */
 	root_user = alloc_uid(ns, 0);
 	if (!root_user) {
-- 
1.6.6

Re: [PATCH 1/1] Syslog are now containerized

[Serge E. Hallyn]

Quoting Jean-Marc Pigeon (jmp-4qkeo2rQ0gg@public.gmane.org):
> 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> 	do not leak in each other.
> 	Running rsyslog daemon within a container won't destroy
> 	host kernel messages.

Thanks Jean-Marc.  But this really isn't doing most of what I'd
recommended in my last emails (both public and private.  In
particular:

> index cc4f453..3d0c73e 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -14,6 +14,7 @@ struct user_namespace {
>  	struct hlist_head	uidhash_table[UIDHASH_SZ];
>  	struct user_struct	*creator;
>  	struct work_struct	destroyer;
> +	struct syslog_ns        *syslog;

syslog_ns should be moved into nsproxy and unshared with a
separate clone(CLONE_SYSLOG);

> -static void emit_log_char(char c)
> +static void emit_log_char(struct syslog_ns *syslog_ns, char c)
>  {
> -	LOG_BUF(log_end) = c;
> -	log_end++;
> -	if (log_end - log_start > log_buf_len)
> -		log_start = log_end - log_buf_len;
> -	if (log_end - con_start > log_buf_len)
> -		con_start = log_end - log_buf_len;
> -	if (logged_chars < log_buf_len)
> -		logged_chars++;
> +	LOG_BUF(syslog_ns, sys_log_end) = c;

Taking syslog_ns from current in emit_log_char is not right.
Emit_log_char() is called from printk which (the comment above
printk warns us) can be called from any context.

That was why I suggested:

>! I think my patch is fundamentally wrong anyway:  we should not
>! use current's context at vprintk like i did.  We should use an
>! optional passed-in context from those callsites where we want to,
>! and default to init otherwise.  That means
>! 
>! 1. put the core of vprintk() into vnsprintk() which takes a syslog
>! namespace as argument
>! 
>! 2. make vprintk() a wrapper around vnsprintk() which passes in
>! init_syslog_ns to vnsprintk().  printk can remain unchanged.
>! 
>! 4. make nsprintk() a wrapper around vnsprintk() which takes a syslog
>! ns argument and pass it to vnsprintk()
>! 
>! 3. do_syslog() can obviously be containerized the same way it
>! is now.
>! 
>! 4. take a printk call like the iptables ones you want and turn
>! int into nsprintk syscall.
>! 
>! The very comment above printk explains why I was an idiot to write
>! let alone send that patch.

thanks,
-serge

Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

Thanks for working on this. Some minor style issues below:

On Thu, Feb 11, 2010 at 01:00:20AM -0500, Jean-Marc Pigeon wrote:
> 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> 	do not leak in each other.
> 	Running rsyslog daemon within a container won't destroy
> 	host kernel messages.
> ---

<snip>

> diff --git a/kernel/syslog.c b/kernel/syslog.c
> new file mode 100644
> index 0000000..69d30a9
> --- /dev/null
> +++ b/kernel/syslog.c
> @@ -0,0 +1,157 @@

<snip>

> +		(void) kfree(old_buf);
> +		}
> +	(void) printk(KERN_NOTICE "log_buf_len: %u\n", syslog_ns->buf_len);

I don't understand why you chose to put in all of the "(void)" casts.
They do nothing -- they don't change how the code works nor do they
improve the readability of the code.

> +	return syslog_ns;
> +}
> +/*
> + * Procedure to free all ressources tied to syslog
> + *
> + */
> +struct syslog_ns *syslog_free(struct syslog_ns *syslog)
> +
> +{
> +	if (syslog != (struct syslog_ns *)0) {

Why aren't you using NULL? Better yet, in these cases it's common to use:

if (syslog) {

Once this gets out of the RFC steps perhaps run these through checkpatch..

Cheers,
	-Matt Helsley

Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

On Thu, Feb 11, 2010 at 11:48:43AM -0600, Serge E. Hallyn wrote:
> Quoting Jean-Marc Pigeon (jmp-4qkeo2rQ0gg@public.gmane.org):
> > 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> > 	do not leak in each other.
> > 	Running rsyslog daemon within a container won't destroy
> > 	host kernel messages.
> 
> Thanks Jean-Marc.  But this really isn't doing most of what I'd
> recommended in my last emails (both public and private.  In
> particular:
> 
> > index cc4f453..3d0c73e 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -14,6 +14,7 @@ struct user_namespace {
> >  	struct hlist_head	uidhash_table[UIDHASH_SZ];
> >  	struct user_struct	*creator;
> >  	struct work_struct	destroyer;
> > +	struct syslog_ns        *syslog;
> 
> syslog_ns should be moved into nsproxy and unshared with a
> separate clone(CLONE_SYSLOG);

I agree. Keeping it in the user_namespace is strange. It should
be in the nsproxy along with all of the other namespace pointers.

> 
> > -static void emit_log_char(char c)
> > +static void emit_log_char(struct syslog_ns *syslog_ns, char c)
> >  {
> > -	LOG_BUF(log_end) = c;
> > -	log_end++;
> > -	if (log_end - log_start > log_buf_len)
> > -		log_start = log_end - log_buf_len;
> > -	if (log_end - con_start > log_buf_len)
> > -		con_start = log_end - log_buf_len;
> > -	if (logged_chars < log_buf_len)
> > -		logged_chars++;
> > +	LOG_BUF(syslog_ns, sys_log_end) = c;
> 
> Taking syslog_ns from current in emit_log_char is not right.
> Emit_log_char() is called from printk which (the comment above
> printk warns us) can be called from any context.
> 
> That was why I suggested:
> 
> >! I think my patch is fundamentally wrong anyway:  we should not
> >! use current's context at vprintk like i did.  We should use an
> >! optional passed-in context from those callsites where we want to,
> >! and default to init otherwise.  That means
> >! 
> >! 1. put the core of vprintk() into vnsprintk() which takes a syslog
> >! namespace as argument
> >! 
> >! 2. make vprintk() a wrapper around vnsprintk() which passes in
> >! init_syslog_ns to vnsprintk().  printk can remain unchanged.
> >! 
> >! 4. make nsprintk() a wrapper around vnsprintk() which takes a syslog
> >! ns argument and pass it to vnsprintk()
> >! 
> >! 3. do_syslog() can obviously be containerized the same way it
> >! is now.
> >! 
> >! 4. take a printk call like the iptables ones you want and turn
> >! int into nsprintk syscall.

Definitely. Actually, I think we can go one step further and say that
passing syslog_ns directly to nsprintk() is wrong too. It's wrong
because we only know the namespace that's relevant to the printk
contents -- and that won't usually be a syslog_ns.

Better to have the "find corresponding syslog_ns" logic inside
nsprintk(). The great thing about doing it this way is if we
ever decide to duplicate printk output to multiple containers it
can be done inside nsprintk rather than doing a flag-day patch.

Cheers,
	-Matt Helsley

Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

On Sat, Feb 13, 2010 at 10:11:58AM -0800, Matt Helsley wrote:
> On Thu, Feb 11, 2010 at 11:48:43AM -0600, Serge E. Hallyn wrote:
> > Quoting Jean-Marc Pigeon (jmp-4qkeo2rQ0gg@public.gmane.org):
> > > 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> > > 	do not leak in each other.
> > > 	Running rsyslog daemon within a container won't destroy
> > > 	host kernel messages.
> > 
> > Thanks Jean-Marc.  But this really isn't doing most of what I'd
> > recommended in my last emails (both public and private.  In
> > particular:
> > 
> > > index cc4f453..3d0c73e 100644
> > > --- a/include/linux/user_namespace.h
> > > +++ b/include/linux/user_namespace.h
> > > @@ -14,6 +14,7 @@ struct user_namespace {
> > >  	struct hlist_head	uidhash_table[UIDHASH_SZ];
> > >  	struct user_struct	*creator;
> > >  	struct work_struct	destroyer;
> > > +	struct syslog_ns        *syslog;
> > 
> > syslog_ns should be moved into nsproxy and unshared with a
> > separate clone(CLONE_SYSLOG);
> 
> I agree. Keeping it in the user_namespace is strange. It should
> be in the nsproxy along with all of the other namespace pointers.
> 
> > 
> > > -static void emit_log_char(char c)
> > > +static void emit_log_char(struct syslog_ns *syslog_ns, char c)
> > >  {
> > > -	LOG_BUF(log_end) = c;
> > > -	log_end++;
> > > -	if (log_end - log_start > log_buf_len)
> > > -		log_start = log_end - log_buf_len;
> > > -	if (log_end - con_start > log_buf_len)
> > > -		con_start = log_end - log_buf_len;
> > > -	if (logged_chars < log_buf_len)
> > > -		logged_chars++;
> > > +	LOG_BUF(syslog_ns, sys_log_end) = c;
> > 
> > Taking syslog_ns from current in emit_log_char is not right.
> > Emit_log_char() is called from printk which (the comment above
> > printk warns us) can be called from any context.
> > 
> > That was why I suggested:
> > 
> > >! I think my patch is fundamentally wrong anyway:  we should not
> > >! use current's context at vprintk like i did.  We should use an
> > >! optional passed-in context from those callsites where we want to,
> > >! and default to init otherwise.  That means
> > >! 
> > >! 1. put the core of vprintk() into vnsprintk() which takes a syslog
> > >! namespace as argument
> > >! 
> > >! 2. make vprintk() a wrapper around vnsprintk() which passes in
> > >! init_syslog_ns to vnsprintk().  printk can remain unchanged.
> > >! 
> > >! 4. make nsprintk() a wrapper around vnsprintk() which takes a syslog
> > >! ns argument and pass it to vnsprintk()
> > >! 
> > >! 3. do_syslog() can obviously be containerized the same way it
> > >! is now.
> > >! 
> > >! 4. take a printk call like the iptables ones you want and turn
> > >! int into nsprintk syscall.
> 
> Definitely. Actually, I think we can go one step further and say that
> passing syslog_ns directly to nsprintk() is wrong too. It's wrong
> because we only know the namespace that's relevant to the printk
> contents -- and that won't usually be a syslog_ns.
> 
> Better to have the "find corresponding syslog_ns" logic inside
> nsprintk(). The great thing about doing it this way is if we
> ever decide to duplicate printk output to multiple containers it
> can be done inside nsprintk rather than doing a flag-day patch.

It's also easy to see that current's nsproxy won't always point to
the right namespace. Consider kernel threads that do "work" which
was initiated by a userspace task (e.g. a process doing a read on
an NFS file triggers rpc IO kthreads). Usually the relevant namespace
pointer will referenced from the kernel objects related to the work --
not "current" since that's the kernel thread. But those structures
don't reference nsproxies -- they refer to a netns for example. So
being able to obtain a syslog_ns from a netns is critical yet can't
rely on current.

Cheers,
	-Matt Helsley

Re: [PATCH 1/1] Syslog are now containerized

[Eric W. Biederman]

Jean-Marc Pigeon <jmp-4qkeo2rQ0gg@public.gmane.org> writes:

> 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> 	do not leak in each other.
> 	Running rsyslog daemon within a container won't destroy
> 	host kernel messages.

If the goal is to not destroy the host kernel messages the much
simpler solution would be to simply disable /proc/kmsg in the container.
I expect we can get that for free with a some bug fixes to the user
namespace (aka if you are not in the global namespace you can't
touch /proc/kmsg).

Additionally except for the possible exception of logging firewall rules
I can't think of a case where I would want kernel printk's in anything
other than the global kernel ring buffer.

Eric

Re: [PATCH 1/1] Syslog are now containerized

[Jean-Marc Pigeon]

Hello,

On Sat, 2010-02-13 at 10:11 -0800, Matt Helsley wrote:
> On Thu, Feb 11, 2010 at 11:48:43AM -0600, Serge E. Hallyn wrote:
> > Quoting Jean-Marc Pigeon (jmp@safe.ca):
> > > 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> > > 	do not leak in each other.
> > > 	Running rsyslog daemon within a container won't destroy
> > > 	host kernel messages.
> > 
> > Thanks Jean-Marc.  But this really isn't doing most of what I'd
> > recommended in my last emails (both public and private.  In
> > particular:
> > 
> > > index cc4f453..3d0c73e 100644
> > > --- a/include/linux/user_namespace.h
> > > +++ b/include/linux/user_namespace.h
> > > @@ -14,6 +14,7 @@ struct user_namespace {
> > >  	struct hlist_head	uidhash_table[UIDHASH_SZ];
> > >  	struct user_struct	*creator;
> > >  	struct work_struct	destroyer;
> > > +	struct syslog_ns        *syslog;
> > 
> > syslog_ns should be moved into nsproxy and unshared with a
> > separate clone(CLONE_SYSLOG);
> 
> I agree. Keeping it in the user_namespace is strange. It should
> be in the nsproxy along with all of the other namespace pointers.
	I won't argue on this, I put it in user_namespace as
	serge proposed it at first and code implementation
	was a test/trial to me.
	Setting syslog in nsproxy is fine to me (seems
	every body agree.. right?)

[...]
> 
> Definitely. Actually, I think we can go one step further and say that
> passing syslog_ns directly to nsprintk() is wrong too. It's wrong
> because we only know the namespace that's relevant to the printk
> contents -- and that won't usually be a syslog_ns.
> 
> Better to have the "find corresponding syslog_ns" logic inside
> nsprintk(). The great thing about doing it this way is if we
> ever decide to duplicate printk output to multiple containers it
> can be done inside nsprintk rather than doing a flag-day patch.

	I tend to agree with you Matt.

	I strongly disagree about what I understand 
	about Serge proposal to add a "new printk".

	What is the purpose of printk?
	- Report a system data to the acting authority.

	Syslog cant be sent to:
	HOST: syslog
	CONT: syslog
	CONT: + HOST:  syslog (duplication)
	Decision to duplicate syslog can be a printk
	privilege, lets says "according message level".

	Decision to send  to HOST: or CONT: syslog
	is depending execution context, which
	can be found back within the printk function
	itself (so no need to have ns_printk).

	What is proposing Serge is to HARD CODE WITHIN kernel
	if we call the genuine printk or ns_printk, which
	is now a door wide open to coder interpretation ("is
	printk message containerised or not", my coding decision
	is as good as yours, and nobody will be satisfied).

	My proposal is to say, if a ressource is containerized
	it must be used in containerized context, this means
	all printk are called within the context anyway.

	What does that mean in real life?. lets call again the iptable
	example.

	We already have a HOST: iptables set AND a CONT: iptables
	set. I have for my say, to keep consistency we MUST
	execute CONT: iptables rules checking within the CONT: context
	(which is not the case now... IMHO).

	This approach is coherent, if a ressource (iptable,
	network device, file system,...) is defined AND managed within
	CONT: report must be done TO CONT: (period). Means
	coding level need to take care ONLY about 'ressource'
	ownership, if it is "own" by CONT:  then access it in its  
	CONT: context. 

	Keep in mind, A fully containerized system can be managed
	by someone with full privilege BUT NOT in charge of 
	the host itself (IE: without host access).

	My proposal is a clear cut, if a ressource is containerized 
	report to CONT: (containerized) syslog... no question ask.
	(sure enough printk could duplicate message to HOST:
	 my guess it won't be really needed in practical life,
	 but I could be wrong....)
	
	
	

-- 
A bientôt
==========================================================================
Jean-Marc Pigeon                                   Internet: jmp@safe.ca
SAFE Inc.                                          Phone: (514) 493-4280
                                                   Fax:   (514) 493-1946
        Clement, 'a kiss solution' to get rid of SPAM (at last)
           Clement' Home base <"http://www.clement.safe.ca">
==========================================================================

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

On Sat, Feb 13, 2010 at 02:14:59PM -0500, Jean-Marc Pigeon wrote:
> Hello,
> 
> On Sat, 2010-02-13 at 10:11 -0800, Matt Helsley wrote:
> > On Thu, Feb 11, 2010 at 11:48:43AM -0600, Serge E. Hallyn wrote:
> > > Quoting Jean-Marc Pigeon (jmp-4qkeo2rQ0gg@public.gmane.org):
> > > > 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> > > > 	do not leak in each other.
> > > > 	Running rsyslog daemon within a container won't destroy
> > > > 	host kernel messages.
> > > 
> > > Thanks Jean-Marc.  But this really isn't doing most of what I'd
> > > recommended in my last emails (both public and private.  In
> > > particular:
> > > 
> > > > index cc4f453..3d0c73e 100644
> > > > --- a/include/linux/user_namespace.h
> > > > +++ b/include/linux/user_namespace.h
> > > > @@ -14,6 +14,7 @@ struct user_namespace {
> > > >  	struct hlist_head	uidhash_table[UIDHASH_SZ];
> > > >  	struct user_struct	*creator;
> > > >  	struct work_struct	destroyer;
> > > > +	struct syslog_ns        *syslog;
> > > 
> > > syslog_ns should be moved into nsproxy and unshared with a
> > > separate clone(CLONE_SYSLOG);
> > 
> > I agree. Keeping it in the user_namespace is strange. It should
> > be in the nsproxy along with all of the other namespace pointers.
> 	I won't argue on this, I put it in user_namespace as
> 	serge proposed it at first and code implementation
> 	was a test/trial to me.
> 	Setting syslog in nsproxy is fine to me (seems
> 	every body agree.. right?)

Good, thanks!

> [...]
> > 
> > Definitely. Actually, I think we can go one step further and say that
> > passing syslog_ns directly to nsprintk() is wrong too. It's wrong
> > because we only know the namespace that's relevant to the printk
> > contents -- and that won't usually be a syslog_ns.
> > 
> > Better to have the "find corresponding syslog_ns" logic inside
> > nsprintk(). The great thing about doing it this way is if we
> > ever decide to duplicate printk output to multiple containers it
> > can be done inside nsprintk rather than doing a flag-day patch.
> 
> 	I tend to agree with you Matt.
> 
> 	I strongly disagree about what I understand 
> 	about Serge proposal to add a "new printk".
> 
> 	What is the purpose of printk?
> 	- Report a system data to the acting authority.
> 
> 	Syslog cant be sent to:
> 	HOST: syslog
> 	CONT: syslog
> 	CONT: + HOST:  syslog (duplication)
> 	Decision to duplicate syslog can be a printk
> 	privilege, lets says "according message level".
> 
> 	Decision to send  to HOST: or CONT: syslog
> 	is depending execution context, which
> 	can be found back within the printk function
> 	itself (so no need to have ns_printk).
> 
> 	What is proposing Serge is to HARD CODE WITHIN kernel
> 	if we call the genuine printk or ns_printk, which
> 	is now a door wide open to coder interpretation ("is
> 	printk message containerised or not", my coding decision
> 	is as good as yours, and nobody will be satisfied).

I think it will be pretty clear from the code which is preferred,
printk or nsprintk. For example, output from the the various module
__init functions obviously does not belong in an nsprintk().

> 
> 	My proposal is to say, if a ressource is containerized
> 	it must be used in containerized context, this means
> 	all printk are called within the context anyway.

But that simply does not happen in all kernel code. Please see my
kernel thread point. Not all work happens in a "containerized
context". Same thing with the __init example above. It doesn't
matter which container caused the module to be loaded -- only
that the kernel has loaded it. So nsprintk() doesn't make sense
there, even though there is the initial namespace to use.

> 	What does that mean in real life?. lets call again the iptable
> 	example.
> 
> 	We already have a HOST: iptables set AND a CONT: iptables
> 	set. I have for my say, to keep consistency we MUST
> 	execute CONT: iptables rules checking within the CONT: context
> 	(which is not the case now... IMHO).

Enforcing this using "current" would require duplicating kernel
threads on a per-container basis. That's simply not going to fly
with the kernel community (and for good reason).

> 
> 	This approach is coherent, if a ressource (iptable,
> 	network device, file system,...) is defined AND managed within

It sounds reasonable except you're hand-waving alot of the complexity
away first.

> 	CONT: report must be done TO CONT: (period). Means
> 	coding level need to take care ONLY about 'ressource'
> 	ownership, if it is "own" by CONT:  then access it in its  
> 	CONT: context.

Tracking all of these accesses down and ensuring they are only done
from "its container context" is difficult or impossible. It's not as
easy as you seem to think. In some cases the same resource could be
shared between containers. Which should we access it from then?

> 	Keep in mind, A fully containerized system can be managed
> 	by someone with full privilege BUT NOT in charge of 
> 	the host itself (IE: without host access).

Sure. (We're not there yet but I think we'd like to get
there eventually.)

> 	My proposal is a clear cut, if a ressource is containerized 
> 	report to CONT: (containerized) syslog... no question ask.

That part of the proposal is simple and makes alot of sense. The
ramifcations of it on kernel code are not simple and often there's
no clean way to do it.

Have a look at the iptables code. I've been looking at it and the
way to make the various printks namespace-clean is far from obvious
without drastically changing the core data structures of iptables.
It's also not obvious that making them namespace clean is
really useful (as Eric seemed to imply). ipt_LOG.c and ipt_ULOG.c are
perhaps the best arguments for nsprintk() in iptables.

> 	(sure enough printk could duplicate message to HOST:
> 	 my guess it won't be really needed in practical life,
> 	 but I could be wrong....)

Yup.

Cheers,
	-Matt Helsley

Re: [PATCH 1/1] Syslog are now containerized

[Jean-Marc Pigeon]

Hello,

[...]
> Tracking all of these accesses down and ensuring they are only done
> from "its container context" is difficult or impossible. It's not as
> easy as you seem to think. In some cases the same resource could be
> shared between containers. Which should we access it from then?

	How come?! ressources (a device, Iptable rules,...)
	containerized within one container could be shared by 
	another unrelated container?.

	Does this means (simple example) someone change
	iptable rules for one container that could change 
	another unrelated container behavior ?!...no way...
	This only case is a sub-container (a container
	within a container), but in such case we are 
	are in the HOST: versus CONT: situation. Device
	will be controlled by CONT: even is used by SUBCONT:
	All depends where the device is defined (where
	is the definition responsability?, that the question
	to assign syslog..., usage is another story).

> 
> > 	Keep in mind, A fully containerized system can be managed
> > 	by someone with full privilege BUT NOT in charge of 
> > 	the host itself (IE: without host access).
> 
> Sure. (We're not there yet but I think we'd like to get
> there eventually.)
> 
> > 	My proposal is a clear cut, if a ressource is containerized 
> > 	report to CONT: (containerized) syslog... no question ask.
> 
> That part of the proposal is simple and makes alot of sense. The
> ramifcations of it on kernel code are not simple and often there's
> no clean way to do it.
	Well, this trouble me somewhat....
	2.6.18-128.2.1.el5.028stab064.7 (just an example, I am using
	day to day), is containerising iptables an other syslogs 
	nice way....,
	We are now 2.6.33 you are telling me what was experimented,
	learned, monthssss ago can't still be implemented 
	in current kernel main stream?.... 


-- 
A bientôt
==========================================================================
Jean-Marc Pigeon                                   Internet: jmp@safe.ca
SAFE Inc.                                          Phone: (514) 493-4280
                                                   Fax:   (514) 493-1946
        Clement, 'a kiss solution' to get rid of SPAM (at last)
           Clement' Home base <"http://www.clement.safe.ca">
==========================================================================

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

On Sat, Feb 13, 2010 at 04:56:16PM -0500, Jean-Marc Pigeon wrote:
> Hello,
> 
> [...]
> > Tracking all of these accesses down and ensuring they are only done
> > from "its container context" is difficult or impossible. It's not as
> > easy as you seem to think. In some cases the same resource could be
> > shared between containers. Which should we access it from then?
> 
> 	How come?! ressources (a device, Iptable rules,...)

Because it's kernel code. Kernel code implements interrupt handlers,
schedules processes, and even does work inside kernel threads. We
can't guarantee that some of the work initiated by a process is
always done with current == <the process>. Hence ensuring that
these accesses are only done from "its container context" is not
reasonable. The best we can do is ensure that accesses from the
processes in the container are contained.

> 	containerized within one container could be shared by 
> 	another unrelated container?.

Yes. namespace boundaries only coincide if userspace chooses to
make them coincide. For example, the tasks in a network namespace
do not necessarily all share the same mount namespace.

> 	Does this means (simple example) someone change
> 	iptable rules for one container that could change 
> 	another unrelated container behavior ?!...no way...

Two "unrelated containers" would share the same iptables rules
so long as they share a network namespace.

<snip>

> > > 	My proposal is a clear cut, if a ressource is containerized 
> > > 	report to CONT: (containerized) syslog... no question ask.
> > 
> > That part of the proposal is simple and makes alot of sense. The
> > ramifcations of it on kernel code are not simple and often there's
> > no clean way to do it.
> 	Well, this trouble me somewhat....
> 	2.6.18-128.2.1.el5.028stab064.7 (just an example, I am using
> 	day to day), is containerising iptables an other syslogs 
> 	nice way....,

Er.. you have a 2.6.18 kernel "containerising iptables an other syslogs"?
I didn't think iptables supported network namespaces until somewhat
recently. Is this an openvz-patched kernel you're talking about?

> 	We are now 2.6.33 you are telling me what was experimented,
> 	learned, monthssss ago can't still be implemented 
> 	in current kernel main stream?.... 

Careful. "no clean way to do it" does not mean "can't be done".

Cheers,
	-Matt Helsley

Re: [PATCH 1/1] Syslog are now containerized

[Jean-Marc Pigeon]

Hello,

On Sat, 2010-02-13 at 14:33 -0800, Matt Helsley wrote:
> On Sat, Feb 13, 2010 at 04:56:16PM -0500, Jean-Marc Pigeon wrote:
> > Hello,
> > 
> > [...]

> 
> Yes. namespace boundaries only coincide if userspace chooses to
> make them coincide. For example, the tasks in a network namespace
> do not necessarily all share the same mount namespace.
> 
> > 	Does this means (simple example) someone change
> > 	iptable rules for one container that could change 
> > 	another unrelated container behavior ?!...no way...
> 
> Two "unrelated containers" would share the same iptables rules
> so long as they share a network namespace.
	So ... logic means.... those two unrelated container
	do not "own" the iptable rules.
	But lets say, for fun, process within container 1
	change rules (locking out ssh access), does it mean
	now ssh connexion on container 2 locked out too...
	If you say "container 0" which container 1 and 2
	are include in, decided to lock ssh access, then
	its OK.
	Container 1 and 2 are still unrelated, right, but both 
	are related  to container 0, and syslog report must 
	go to container 0.
	(once again it is clean cut.)
	
[...]
	

> > > That part of the proposal is simple and makes alot of sense. The
> > > ramifcations of it on kernel code are not simple and often there's
> > > no clean way to do it.
> > 	Well, this trouble me somewhat....
> > 	2.6.18-128.2.1.el5.028stab064.7 (just an example, I am using
> > 	day to day), is containerising iptables an other syslogs 
> > 	nice way....,
> 
> Er.. you have a 2.6.18 kernel "containerising iptables an other syslogs"?
> I didn't think iptables supported network namespaces until somewhat
> recently. Is this an openvz-patched kernel you're talking about?

	Yep! release date 07-Nov-2009, and I am pretty sure
	2.6.18-53.1.19.el5.028stab053.14 release date 21-May-2008 
	was doing it too...

	Iptable logs are reported to VZ (I have an example
	right in front of me)

Feb 13 14:42:13 host1 kernel: RJCT IN=venet0 OUT= MAC= SRC=X.X.X.X
DST=X.X.X.X LEN=60 TOS=0x00 PREC=0x00 TTL=64 ID=58325 DF PROTO=TCP
SPT=37248 DPT=22 WINDOW=5840 RES=0x00 SYN URGP=0

	When I said monthssss, I really mean it.

> Careful. "no clean way to do it" does not mean "can't be done".
	Agreed....container network, seems to me, implemented
	in far better way than on VZ, so it is possible to implement 
	good idea in clean way.
	

-- 
A bientôt
==========================================================================
Jean-Marc Pigeon                                   Internet: jmp@safe.ca
SAFE Inc.                                          Phone: (514) 493-4280
                                                   Fax:   (514) 493-1946
        Clement, 'a kiss solution' to get rid of SPAM (at last)
           Clement' Home base <"http://www.clement.safe.ca">
==========================================================================

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [PATCH 1/1] Syslog are now containerized

[Jean-Marc Pigeon]

Hello,

	Just got your Email... (Sic :-})

On Sat, 2010-02-13 at 11:13 -0800, Eric W. Biederman wrote:
> Jean-Marc Pigeon <jmp@safe.ca> writes:
> 
> > 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> > 	do not leak in each other.
> > 	Running rsyslog daemon within a container won't destroy
> > 	host kernel messages.
> 
> If the goal is to not destroy the host kernel messages the much
> simpler solution would be to simply disable /proc/kmsg in the container.
> I expect we can get that for free with a some bug fixes to the user
> namespace (aka if you are not in the global namespace you can't
> touch /proc/kmsg).
> 
> Additionally except for the possible exception of logging firewall rules
> I can't think of a case where I would want kernel printk's in anything
> other than the global kernel ring buffer.

	Beside not to have HOST: syslog corrupted, my very original main
	concern was indeed to feed container with its own firewall
	rules.

	Thinking about all this, I believe we are not bold enough.
	We should be reporting all kernel message about devices/units
	own/defined within the container to the own container syslog.

	Let me try explain better by an example. To make container
	networking you can use veth pair.
	One of the veth pair is given to container and related
	to container own network definition (eth0).

	this TACAMO order "ip link set 'from_cont_veth' netns..."
	make now the container "Take ChArge and Move Out" and
	all kernel trouble to have the interface fully working
	within the container should be reported to container
	syslog.

	Keep in mind, CONT: sys-admin could be a different
	person than HOST: sys-admin. As long veth pair
	is set properly, CONT: sys-admin problem setting
	with eth0 is not a HOST: sys-admin concern.

	A fully working container syslog will address/resolve this
	kind of situation.


	

	
-- 
A bientôt
==========================================================================
Jean-Marc Pigeon                                   Internet: jmp@safe.ca
SAFE Inc.                                          Phone: (514) 493-4280
                                                   Fax:   (514) 493-1946
        Clement, 'a kiss solution' to get rid of SPAM (at last)
           Clement' Home base <"http://www.clement.safe.ca">
==========================================================================

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers