linux-kernel - Re: Regression - /proc/kmsg does not (always) block for 1-byte reads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1341713399.22696.9.camel@mop>
Date:	Sun, 08 Jul 2012 04:09:59 +0200
From:	Kay Sievers <kay@...y.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Greg Kroah-Hartman <greg@...ah.com>,
	Jukka Ollila <jiiksteri@...il.com>,
	linux-kernel@...r.kernel.org, jbeulich@...ell.com,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: Regression - /proc/kmsg does not (always) block for 1-byte reads

On Sat, 2012-07-07 at 23:19 +0200, Kay Sievers wrote:
> On Fri, Jul 6, 2012 at 10:30 PM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> > Kay, this needs to be fixed.
> >
> > Suggested fix: just use the 'seq_printf()' interfaces, which do the
> > proper buffering, and allow any size reads of various packetized data.
> 
> I'll have a look.

Hmm, we need to block in the read() when we have no data, and we need to
support concurrent readers where only one of them sees the data, and we
need O_NONBLOCK support. Maybe I miss something but the seq_file stuff
seems to get complicated, as it takes a mutex internally which gets in
the way of the O_NONBLOCK stuff.

Here is what seems to work for me. If the buffer is to small to fit the
first record, we deliver a partial record, and start from that offset
again with the next read().

I'll need to do more testing tomorrow.

Thanks,
Kay

---
 kernel/printk.c |   28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -217,6 +217,7 @@ static DEFINE_RAW_SPINLOCK(logbuf_lock);
 /* the next printk record to read by syslog(READ) or /proc/kmsg */
 static u64 syslog_seq;
 static u32 syslog_idx;
+static size_t syslog_partial;
 
 /* index and sequence number of the first record stored in the buffer */
 static u64 log_first_seq;
@@ -890,22 +891,33 @@ static int syslog_print(char __user *buf
 
 	while (size > 0) {
 		size_t n;
+		size_t skip;
 
 		raw_spin_lock_irq(&logbuf_lock);
 		if (syslog_seq < log_first_seq) {
 			/* messages are gone, move to first one */
 			syslog_seq = log_first_seq;
 			syslog_idx = log_first_idx;
+			syslog_partial = 0;
 		}
 		if (syslog_seq == log_next_seq) {
 			raw_spin_unlock_irq(&logbuf_lock);
 			break;
 		}
+
+		skip = syslog_partial;
 		msg = log_from_idx(syslog_idx);
 		n = msg_print_text(msg, true, text, LOG_LINE_MAX);
-		if (n <= size) {
+		if (n - syslog_partial <= size) {
+			/* message fits into buffer, move forward */
 			syslog_idx = log_next(syslog_idx);
 			syslog_seq++;
+			n -= syslog_partial;
+			syslog_partial = 0;
+		} else if (!len){
+			/* partial read(), remember position */
+			n = size;
+			syslog_partial += n;
 		} else
 			n = 0;
 		raw_spin_unlock_irq(&logbuf_lock);
@@ -913,17 +925,15 @@ static int syslog_print(char __user *buf
 		if (!n)
 			break;
 
-		len += n;
-		size -= n;
-		buf += n;
-		n = copy_to_user(buf - n, text, n);
-
-		if (n) {
-			len -= n;
+		if (copy_to_user(buf, text + skip, n)) {
 			if (!len)
 				len = -EFAULT;
 			break;
 		}
+
+		len += n;
+		size -= n;
+		buf += n;
 	}
 
 	kfree(text);
@@ -1107,6 +1117,7 @@ int do_syslog(int type, char __user *buf
 			/* messages are gone, move to first one */
 			syslog_seq = log_first_seq;
 			syslog_idx = log_first_idx;
+			syslog_partial = 0;
 		}
 		if (from_file) {
 			/*
@@ -1129,6 +1140,7 @@ int do_syslog(int type, char __user *buf
 				idx = log_next(idx);
 				seq++;
 			}
+			error -= syslog_partial;
 		}
 		raw_spin_unlock_irq(&logbuf_lock);
 		break;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/