Re: [RFC PATCH] cmdline: Hide "debug" from /proc/cmdline

[Linus Torvalds]

On Wed, Apr 2, 2014 at 4:52 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> TOTALLY UNTESTED. But it really isn't complex.

Oh, and here's a patch that is actually lightly tested. I did

    while :; do echo hello; done > /dev/kmsg

(the 'yes' program buffers output, so won't work) and I get

    [  122.062912] hello
    [  122.062915] hello
    [  122.062918] hello
    [  122.062921] hello
    [  122.062924] hello
    [  122.062927] hello
    [  122.062930] hello
    [  122.062932] hello
    [  122.062935] hello
    [  122.062938] hello
    [  127.062671] bash: 2104439 callbacks suppressed

so it works (repeating every five seconds, as expected).

It's definitely not perfect - if we suppress output, and the process
then closes the file descriptor rather than continuing to write more,
you won't  get that "suppressed" message. But it's a usable starting
point for testing and commentary on the actual limits.

So we should probably add reporting about suppressed messages at file
close time, and we should tweak the limits (for example, perhaps not
limit things if the buffers are largely empty - which happens at
bootup), but on the whole I think this is a reasonable thing to do.

Whether it actually fixes the problem that Borislav had is
questionable, of course. For all I know, systemd debug mode generates
so much data in *other* ways and then causes feedback loops with the
kernel debugging that this patch is totally immaterial, and dmesg was
never the main issue. But unlike the "hide 'debug' from
/proc/cmdline", I think this patch at least _conceptually_ makes a lot
of sense, even if systemd gets fixed, so ...

Borislav?

                Linus
 kernel/printk/printk.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 4dae9cbe9259..b01ba10fb1b9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -410,6 +410,7 @@ struct devkmsg_user {
 	u64 seq;
 	u32 idx;
 	enum log_flags prev;
+	struct ratelimit_state rs;
 	struct mutex lock;
 	char buf[8192];
 };
@@ -421,11 +422,15 @@ static ssize_t devkmsg_writev(struct kiocb *iocb, const struct iovec *iv,
 	int i;
 	int level = default_message_loglevel;
 	int facility = 1;	/* LOG_USER */
+	struct file *file = iocb->ki_filp;
+	struct devkmsg_user *user = file->private_data;
 	size_t len = iov_length(iv, count);
 	ssize_t ret = len;
 
-	if (len > LOG_LINE_MAX)
+	if (!user || len > LOG_LINE_MAX)
 		return -EINVAL;
+	if (!___ratelimit(&user->rs, current->comm))
+		return ret;
 	buf = kmalloc(len+1, GFP_KERNEL);
 	if (buf == NULL)
 		return -ENOMEM;
@@ -656,21 +661,22 @@ static unsigned int devkmsg_poll(struct file *file, poll_table *wait)
 static int devkmsg_open(struct inode *inode, struct file *file)
 {
 	struct devkmsg_user *user;
-	int err;
-
-	/* write-only does not need any file context */
-	if ((file->f_flags & O_ACCMODE) == O_WRONLY)
-		return 0;
 
-	err = check_syslog_permissions(SYSLOG_ACTION_READ_ALL,
-				       SYSLOG_FROM_READER);
-	if (err)
-		return err;
+	/* write-only does not need to check read permissions */
+	if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
+		int err = check_syslog_permissions(SYSLOG_ACTION_READ_ALL,
+					       SYSLOG_FROM_READER);
+		if (err)
+			return err;
+	}
 
 	user = kmalloc(sizeof(struct devkmsg_user), GFP_KERNEL);
 	if (!user)
 		return -ENOMEM;
 
+	/* Configurable? */
+	ratelimit_state_init(&user->rs, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST);
+
 	mutex_init(&user->lock);
 
 	raw_spin_lock_irq(&logbuf_lock);