linux-kernel - [PATCH] Revert "kernel/printk: add kmsg SEEK

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-next>] [day] [month] [year] [list]

Message-Id: <20200622030222.1370098-1-Jason@zx2c4.com>
Date:   Sun, 21 Jun 2020 21:02:22 -0600
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     pmladek@...e.com, linux-kernel@...r.kernel.org
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Bruno Meneguele <bmeneg@...hat.com>,
        sergey.senozhatsky@...il.com, rostedt@...dmis.org,
        David.Laight@...LAB.COM,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH] Revert "kernel/printk: add kmsg SEEK_CUR handling"

This reverts commit 8ece3b3eb576a78d2e67ad4c3a80a39fa6708809.

This commit broke userspace. Bash uses ESPIPE to determine whether or
not the file should be read using "unbuffered I/O", which means reading
1 byte at a time instead of 128 bytes at a time. I used to use bash to
read through kmsg in a really quite nasty way:

    while read -t 0.1 -r line 2>/dev/null || [[ $? -ne 142 ]]; do
       echo "SARU $line"
    done < /dev/kmsg

This will show all lines that can fit into the 128 byte buffer, and skip
lines that don't. That's pretty awful, but at least it worked.

With this change, bash now tries to do 1-byte reads, which means it
skips all the lines, which is worse than before.

Now, I don't really care very much about this, and I'm already look for
a workaround. But I did just spend an hour trying to figure out why my
scripts were broken. Either way, it makes no difference to me personally
whether this is reverted, but it might be something to consider. If you
declare that "trying to read /dev/kmsg with bash is terminally stupid
anyway," I might be inclined to agree with you. But do note that bash
uses lseek(fd, 0, SEEK_CUR)==>ESPIPE to determine whether or not it's
reading from a pipe.

Cc: Bruno Meneguele <bmeneg@...hat.com>
Cc: pmladek@...e.com
Cc: sergey.senozhatsky@...il.com
Cc: rostedt@...dmis.org
Cc: David.Laight@...LAB.COM
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc: Petr Mladek <pmladek@...e.com>
Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
---
 Documentation/ABI/testing/dev-kmsg |  5 -----
 kernel/printk/printk.c             | 10 ----------
 2 files changed, 15 deletions(-)

diff --git a/Documentation/ABI/testing/dev-kmsg b/Documentation/ABI/testing/dev-kmsg
index 1e6c28b1942b..f307506eb54c 100644
--- a/Documentation/ABI/testing/dev-kmsg
+++ b/Documentation/ABI/testing/dev-kmsg
@@ -56,11 +56,6 @@ Description:	The /dev/kmsg character device node provides userspace access
 		  seek after the last record available at the time
 		  the last SYSLOG_ACTION_CLEAR was issued.
 
-		Due to the record nature of this interface with a "read all"
-		behavior and the specific positions each seek operation sets,
-		SEEK_CUR is not supported, returning -ESPIPE (invalid seek) to
-		errno whenever requested.
-
 		The output format consists of a prefix carrying the syslog
 		prefix including priority and facility, the 64 bit message
 		sequence number and the monotonic timestamp in microseconds,
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8c14835be46c..b71eaf5f5a86 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -974,16 +974,6 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
 		user->idx = log_next_idx;
 		user->seq = log_next_seq;
 		break;
-	case SEEK_CUR:
-		/*
-		 * It isn't supported due to the record nature of this
-		 * interface: _SET _DATA and _END point to very specific
-		 * record positions, while _CUR would be more useful in case
-		 * of a byte-based log. Because of that, return the default
-		 * errno value for invalid seek operation.
-		 */
-		ret = -ESPIPE;
-		break;
 	default:
 		ret = -EINVAL;
 	}
-- 
2.27.0