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

[Serge E. Hallyn]

Quoting Jean-Marc Pigeon (jmp at 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);

> -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
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers