2002-08-14 03:04:10

by Andrew Morton

[permalink] [raw]
Subject: [patch] printk from userspace


The patch allows userspace to issue printk's, via sys_syslog():

#include <sys/klog.h>

int do_syslog(char *msg)
{
return klogctl(10, msg, strlen(msg));
}

main()
{
char big[2000];

do_syslog("<1>one\n");
do_syslog("<2>two");
do_syslog("<3>three\n");
memset(big, 'a', sizeof(big));
big[1999] = 0;
do_syslog(big);
}


The main use of this is within hpa's klibc - initial userspace needs a
way of logging information and this API allows that information to be
captured into the printk ringbuffer. It ends up in /var/log/messages.

Messages are truncated at 1024 characters by printk's vsprintf().

Requires CAP_SYS_ADMIN.


printk.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+)

--- 2.5.31/kernel/printk.c~hpa-printk Tue Aug 13 20:15:32 2002
+++ 2.5.31-akpm/kernel/printk.c Tue Aug 13 20:15:32 2002
@@ -25,6 +25,7 @@
#include <linux/module.h>
#include <linux/interrupt.h> /* For in_interrupt() */
#include <linux/config.h>
+#include <linux/slab.h>
#include <linux/delay.h>

#include <asm/uaccess.h>
@@ -163,12 +164,15 @@ __setup("console=", console_setup);
* 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 -- Printk from userspace. Includes loglevel. Returns number of
+ * chars printed.
*/
int do_syslog(int type, char * buf, int len)
{
unsigned long i, j, limit, count;
int do_clear = 0;
char c;
+ char *lbuf = NULL;
int error = 0;

switch (type) {
@@ -283,11 +287,23 @@ int do_syslog(int type, char * buf, int
error = log_end - log_start;
spin_unlock_irq(&logbuf_lock);
break;
+ case 10:
+ lbuf = kmalloc(len + 1, GFP_KERNEL);
+ error = -ENOMEM;
+ if (lbuf == NULL)
+ break;
+ error = -EFAULT;
+ if (copy_from_user(lbuf, buf, len))
+ break;
+ lbuf[len] = '\0';
+ error = printk("%s", lbuf);
+ break;
default:
error = -EINVAL;
break;
}
out:
+ kfree(lbuf);
return error;
}


.


2002-08-14 03:57:03

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Benjamin LaHaise wrote:
> On Tue, Aug 13, 2002 at 08:18:18PM -0700, Andrew Morton wrote:
>
>>The patch allows userspace to issue printk's, via sys_syslog():
>
>
> This is an incredibly bad idea. It has security hole written all over it.
> Any user can now spam the kernel's log ringbuffer and overrun potentially
> important messages.
>

First of all, only CAP_SYS_ADMIN. As far as spamming the ring buffer,
that's trivial to do today by just sending a bunch of bad network
packets, or attaching a USB CD-ROM without a disc in the drive (yes,
really... on my wife's laptop it was so bad that unless she unplugged
the CD-ROM syslogd was eating her system alive), or...

-hpa


2002-08-14 03:57:50

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Benjamin LaHaise wrote:
>
> On Tue, Aug 13, 2002 at 08:18:18PM -0700, Andrew Morton wrote:
> >
> > The patch allows userspace to issue printk's, via sys_syslog():
>
> This is an incredibly bad idea. It has security hole written all over it.
> Any user can now spam the kernel's log ringbuffer and overrun potentially
> important messages.
>

It requires root.

2002-08-14 03:54:11

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Tue, Aug 13, 2002 at 08:18:18PM -0700, Andrew Morton wrote:
>
> The patch allows userspace to issue printk's, via sys_syslog():

This is an incredibly bad idea. It has security hole written all over it.
Any user can now spam the kernel's log ringbuffer and overrun potentially
important messages.

-ben

2002-08-14 04:03:36

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Tue, Aug 13, 2002 at 09:11:55PM -0700, Andrew Morton wrote:
> It requires root.

Still, unlike kernel code that can be rate limited, this call cannot.
Besides, isn't adding yet another syscall that's equivalent to write(2)
a reason to take this patch and burn it along with the vomit its caused?

-ben
--
"You will be reincarnated as a toad; and you will be much happier."

2002-08-14 04:01:55

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Tue, Aug 13, 2002 at 09:00:38PM -0700, H. Peter Anvin wrote:
> First of all, only CAP_SYS_ADMIN. As far as spamming the ring buffer,
> that's trivial to do today by just sending a bunch of bad network
> packets,

Not true, network logging is rate limited.

> or attaching a USB CD-ROM without a disc in the drive (yes,
> really... on my wife's laptop it was so bad that unless she unplugged
> the CD-ROM syslogd was eating her system alive), or...

Well, that's more of a bug and requires console access anyways.

-ben
--
"You will be reincarnated as a toad; and you will be much happier."

2002-08-14 04:07:03

by Alexander Viro

[permalink] [raw]
Subject: Re: [patch] printk from userspace



On Tue, 13 Aug 2002, Benjamin LaHaise wrote:

> On Tue, Aug 13, 2002 at 08:18:18PM -0700, Andrew Morton wrote:
> >
> > The patch allows userspace to issue printk's, via sys_syslog():
>
> This is an incredibly bad idea. It has security hole written all over it.
> Any user can now spam the kernel's log ringbuffer and overrun potentially
> important messages.

He does capability checks there...

2002-08-14 04:16:30

by Alexander Viro

[permalink] [raw]
Subject: Re: [patch] printk from userspace



On Wed, 14 Aug 2002, Benjamin LaHaise wrote:

> On Tue, Aug 13, 2002 at 09:11:55PM -0700, Andrew Morton wrote:
> > It requires root.
>
> Still, unlike kernel code that can be rate limited, this call cannot.
> Besides, isn't adding yet another syscall that's equivalent to write(2)
> a reason to take this patch and burn it along with the vomit its caused?

I have a better suggestion. How about we make write(2) on /dev/console to
act as printk()? IOW, how about making _all_ writes to console show up in
dmesg?

Then we don't need anything special to do logging _and_ we get output
of init scripts captured. For free. dmesg(8) would pick that up, klogd(8)
will work as is, etc.

Linus?

2002-08-14 04:20:45

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch] printk from userspace


On Wed, 14 Aug 2002, Alexander Viro wrote:
>
>
> On Wed, 14 Aug 2002, Benjamin LaHaise wrote:
>
> > On Tue, Aug 13, 2002 at 09:11:55PM -0700, Andrew Morton wrote:
> > > It requires root.
> >
> > Still, unlike kernel code that can be rate limited, this call cannot.
> > Besides, isn't adding yet another syscall that's equivalent to write(2)
> > a reason to take this patch and burn it along with the vomit its caused?
>
> I have a better suggestion. How about we make write(2) on /dev/console to
> act as printk()? IOW, how about making _all_ writes to console show up in
> dmesg?
>
> Then we don't need anything special to do logging _and_ we get output
> of init scripts captured. For free. dmesg(8) would pick that up, klogd(8)
> will work as is, etc.
>
> Linus?

Well, apart from the fact that you get into an endless loop with klogd,
who wants to spew the kernel messages back to the console under some
circumstances..

That said, I like the notion. I've always hated the fact that all the
boot-time messages get lost, simply because syslogd hadn't started, and
as a result things like fsck ran without any sign afterwards. The kernel
log approach saves it all in one place.

But /dev/console just sounds potentially _too_ noisy.

Linus

2002-08-14 04:31:14

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Tue, Aug 13, 2002 at 09:26:45PM -0700, Linus Torvalds wrote:
> That said, I like the notion. I've always hated the fact that all the
> boot-time messages get lost, simply because syslogd hadn't started, and
> as a result things like fsck ran without any sign afterwards. The kernel
> log approach saves it all in one place.
>
> But /dev/console just sounds potentially _too_ noisy.

/dev/kmsg was another suggestion for the name. But please revert the
yet-another-syscall variant -- having a duplicate way for logging that
doesn't work with stdio just seems sick to me (sys_syslog should die).
Something like the following untested code is much better.

-ben
--
"You will be reincarnated as a toad; and you will be much happier."


diff -urN v2.5.31/drivers/char/mem.c foo/drivers/char/mem.c
--- v2.5.31/drivers/char/mem.c Fri Aug 2 11:41:29 2002
+++ foo/drivers/char/mem.c Wed Aug 14 00:34:33 2002
@@ -579,6 +579,24 @@
write: write_full,
};

+static ssize_t kmsg_write(struct file * file, const char * buf,
+ size_t count, loff_t *ppos)
+{
+ char tmp[1025];
+
+ if (count > 1024)
+ count = 1024;
+ if (copy_from_user(tmp, buf, count))
+ return -EFAULT;
+ tmp[count] = 0;
+ printk("%s", tmp);
+ return count;
+}
+
+static struct file_operations kmsg_fops = {
+ write: kmsg_write,
+};
+
static int memory_open(struct inode * inode, struct file * filp)
{
switch (minor(inode->i_rdev)) {
@@ -608,6 +626,9 @@
case 9:
filp->f_op = &urandom_fops;
break;
+ case 11:
+ filp->f_op = &kmsg_fops;
+ break;
default:
return -ENXIO;
}
@@ -634,7 +655,8 @@
{5, "zero", S_IRUGO | S_IWUGO, &zero_fops},
{7, "full", S_IRUGO | S_IWUGO, &full_fops},
{8, "random", S_IRUGO | S_IWUSR, &random_fops},
- {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops}
+ {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops},
+ {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops},
};
int i;

2002-08-14 04:29:41

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Alexander Viro wrote:
>
> I have a better suggestion. How about we make write(2) on /dev/console to
> act as printk()? IOW, how about making _all_ writes to console show up in
> dmesg?
>
> Then we don't need anything special to do logging _and_ we get output
> of init scripts captured. For free. dmesg(8) would pick that up, klogd(8)
> will work as is, etc.
>

/dev/console is probably unsuitable for this (/dev/console is an
interactive device), but something like /dev/kmsg would probably be a
good idea -- it can also replace /proc/kmsg.

However, Andrew's sys_syslog() change does what I need, since users are
used to calling syslog(3) to log messages anyway.

-hpa


2002-08-14 04:38:19

by Linus Torvalds

[permalink] [raw]
Subject: Re: [patch] printk from userspace


On Wed, 14 Aug 2002, Benjamin LaHaise wrote:
>
> /dev/kmsg was another suggestion for the name. But please revert the
> yet-another-syscall variant -- having a duplicate way for logging that
> doesn't work with stdio just seems sick to me (sys_syslog should die).

Actually, anybody who uses stdio on syslog messages should be roasted.
Over the nice romantic glow of red-hot coal, slowly cooking the stupid git
alive.

It's not a bug, it's a feature. A syslog message needs to be atomic, which
means that it MUST NOT use the buffering of stdio.

Linus

2002-08-14 04:41:12

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Linus Torvalds wrote:
> On Wed, 14 Aug 2002, Benjamin LaHaise wrote:
>
>>/dev/kmsg was another suggestion for the name. But please revert the
>>yet-another-syscall variant -- having a duplicate way for logging that
>>doesn't work with stdio just seems sick to me (sys_syslog should die).
>
>
> Actually, anybody who uses stdio on syslog messages should be roasted.
> Over the nice romantic glow of red-hot coal, slowly cooking the stupid git
> alive.
>
> It's not a bug, it's a feature. A syslog message needs to be atomic, which
> means that it MUST NOT use the buffering of stdio.
>

You can do stdio nonbuffered. As a matter of fact, if you're using
klogd, it *will* be nonbuffered :^)

The point that Ben is making is that it should be a write() system call
instead of something ad hockish, and I think I have to agree with him --
although, again, Andrew's patch does what I need.

-hpa


2002-08-14 04:39:15

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Benjamin LaHaise wrote:
> On Tue, Aug 13, 2002 at 09:26:45PM -0700, Linus Torvalds wrote:
>
>>That said, I like the notion. I've always hated the fact that all the
>>boot-time messages get lost, simply because syslogd hadn't started, and
>>as a result things like fsck ran without any sign afterwards. The kernel
>>log approach saves it all in one place.
>>
>>But /dev/console just sounds potentially _too_ noisy.
>
>
> /dev/kmsg was another suggestion for the name. But please revert the
> yet-another-syscall variant -- having a duplicate way for logging that
> doesn't work with stdio just seems sick to me (sys_syslog should die).
> Something like the following untested code is much better.
>

a) /dev/kmsg bettwe be S_IRUSR|S_IWUSR... reading /{proc,dev}/kmsg
should drain the ring buffer.

b) Hook up the /proc/kmsg read to this thing.

It really needs to be a /dev node, not a /proc node; procfs is likely
*not* to be mounted; however, manifesting a /dev node is easy enough.

-hpa


2002-08-14 04:44:15

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Tue, Aug 13, 2002 at 09:44:14PM -0700, Linus Torvalds wrote:
> Actually, anybody who uses stdio on syslog messages should be roasted.
> Over the nice romantic glow of red-hot coal, slowly cooking the stupid git
> alive.

If you're logging huge messages, sure, that's just plain Stupid. But for
messages that are smaller than the size of the stdio buffer an fprintf()
followed by fflush() gets a single atomic write for most values of libc.

> It's not a bug, it's a feature. A syslog message needs to be atomic, which
> means that it MUST NOT use the buffering of stdio.

And that's why we have write(2) on file descriptors. Having write(2)
in the form of syslog(2) makes no sense. It adds to the mass of abi
that needs to be maintained. Making the mechanism of writing using the
existing infrastructure doesn't increase the size of the ABI.

-ben
--
"You will be reincarnated as a toad; and you will be much happier."

2002-08-14 04:44:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Benjamin LaHaise wrote:
>
> ...
> Something like the following untested code is much better.

I agree. And it worked first time.


--- 2.5.31/drivers/char/mem.c~bcrl-printk Tue Aug 13 21:53:24 2002
+++ 2.5.31-akpm/drivers/char/mem.c Tue Aug 13 21:53:34 2002
@@ -579,6 +579,24 @@ static struct file_operations full_fops
write: write_full,
};

+static ssize_t kmsg_write(struct file * file, const char * buf,
+ size_t count, loff_t *ppos)
+{
+ char tmp[1025];
+
+ if (count > 1024)
+ count = 1024;
+ if (copy_from_user(tmp, buf, count))
+ return -EFAULT;
+ tmp[count] = 0;
+ printk("%s", tmp);
+ return count;
+}
+
+static struct file_operations kmsg_fops = {
+ write: kmsg_write,
+};
+
static int memory_open(struct inode * inode, struct file * filp)
{
switch (minor(inode->i_rdev)) {
@@ -608,6 +626,9 @@ static int memory_open(struct inode * in
case 9:
filp->f_op = &urandom_fops;
break;
+ case 11:
+ filp->f_op = &kmsg_fops;
+ break;
default:
return -ENXIO;
}
@@ -634,7 +655,8 @@ void __init memory_devfs_register (void)
{5, "zero", S_IRUGO | S_IWUGO, &zero_fops},
{7, "full", S_IRUGO | S_IWUGO, &full_fops},
{8, "random", S_IRUGO | S_IWUSR, &random_fops},
- {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops}
+ {9, "urandom", S_IRUGO | S_IWUSR, &urandom_fops},
+ {11,"kmsg", S_IRUGO | S_IWUSR, &kmsg_fops},
};
int i;


.

2002-08-14 05:28:28

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] printk from userspace

Alright, klibc 0.35 uses /dev/kmsg for syslog(3).

-hpa

2002-08-14 07:55:26

by Miquel van Smoorenburg

[permalink] [raw]
Subject: Re: [patch] printk from userspace

In article <[email protected]>,
Linus Torvalds <[email protected]> wrote:
>That said, I like the notion. I've always hated the fact that all the
>boot-time messages get lost, simply because syslogd hadn't started, and
>as a result things like fsck ran without any sign afterwards. The kernel
>log approach saves it all in one place.

I have a bootlogd that does a TIOCCONS on /dev/console, so
that it can capture all messages written to /dev/console.

It buffers the messages in-memory, until it is able to open
a logfile in /var/log/ at which point it writes the buffered
data to the logfile, and starts logging to that file.

The only problem is that TIOCCONS is a redirect, so there's no
output to the console anymore. Ofcourse that can be solved by
letting bootlogd open("/dev/realconsole") and sending a copy
to it, but there is no way to ask the kernel to which _real_
device /dev/console is connected.

I submitted a TIOCGDEV ioctl patch a few times during 2.2 development
but it was never integrated, alas.

So this is all solveable in userspace. No need to buffer
messages in non-swappable memory in the the kernel.

Simply add TIOCGDEV or add a flags to the TIOCCONS ioctl that
means 'copy instead of redirect'. Both are useful .. Or, hmm,
interesting, add some code so that if you write to the master
side of the pty pair to which the console is redirected, the
output ends up on the real console. That has a nice symmetric
feel to it.

Sample code is in sysvinit since 2.79 or 2.80, sysvinit-2.xx/src/bootlogd.c

Mike.

2002-08-15 11:30:57

by Arkadiusz Miskiewicz

[permalink] [raw]
Subject: serial console (was Re: [patch] printk from userspace)

"Miquel van Smoorenburg" <[email protected]> writes:

> I have a bootlogd that does a TIOCCONS on /dev/console, so
> that it can capture all messages written to /dev/console.
>
> It buffers the messages in-memory, until it is able to open
> a logfile in /var/log/ at which point it writes the buffered
> data to the logfile, and starts logging to that file.
>
> The only problem is that TIOCCONS is a redirect, so there's no
> output to the console anymore.
btw. is there any reason to not display initscripts messages
on all consoles in such setup:
append="console=tty0 console=ttyS0,57600n8"
serial=0,57600n8
?

kernel messages are available on both - tty0 and ttyS0 while
userspace messages (from initscripts) only on last specified
- in such case ttyS0.

> Mike.

--
Arkadiusz Mi?kiewicz IPv6 ready PLD Linux at http://www.pld.org.pl
misiek(at)pld.org.pl AM2-6BONE, 1024/3DB19BBD, arekm(at)ircnet, PWr

2002-08-15 12:39:46

by Miquel van Smoorenburg

[permalink] [raw]
Subject: Re: serial console (was Re: [patch] printk from userspace)

In article <[email protected]>,
Arkadiusz Miskiewicz <[email protected]> wrote:
>btw. is there any reason to not display initscripts messages
>on all consoles in such setup:
>append="console=tty0 console=ttyS0,57600n8"
>serial=0,57600n8
>?

Yes. It is not implemented in the kernel and it would not
be trivial to do so (or even wanted)

>kernel messages are available on both - tty0 and ttyS0 while
>userspace messages (from initscripts) only on last specified
>- in such case ttyS0.

That's because printk() in the kernel uses a very different codepath than
the userland write(console, "hello")

Mike.

2002-08-19 11:24:26

by Daniel Phillips

[permalink] [raw]
Subject: Re: [patch] printk from userspace

On Wednesday 14 August 2002 07:32, H. Peter Anvin wrote:
> Alright, klibc 0.35 uses /dev/kmsg for syslog(3).

Could we please have '/dev/kmessage' instead of 'kmsg'?

--
Daniel