[PATCH v2] kernel/printk: add kmsg SEEK_CUR handling

[PATCH v2] kernel/printk: add kmsg SEEK_CUR handling

[Bruno Meneguele]

Userspace libraries, e.g. glibc's dprintf(), perform a SEEK_CUR operation
over any file descriptor requested to make sure the current position isn't
pointing to junk due to previous manipulation of that same fd. And whenever
that fd doesn't have support for such operation, the userspace code expects
-ESPIPE to be returned.

However, when the fd in question references the /dev/kmsg interface, the
current kernel code state returns -EINVAL instead, causing an unexpected
behavior in userspace: in the case of glibc, when -ESPIPE is returned it
gets ignored and the call completes successfully, while returning -EINVAL
forces dprintf to fail without performing any action over that fd:

  if (_IO_SEEKOFF (fp, (off64_t)0, _IO_seek_cur, _IOS_INPUT|_IOS_OUTPUT) ==
  _IO_pos_BAD && errno != ESPIPE)
    return NULL;

With this patch we make sure to return the correct value when SEEK_CUR is
requested over kmsg and also add some kernel doc information to formalize
this behavior.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
---
v2: add more documentation details to code, kernel doc and patch log

 Documentation/ABI/testing/dev-kmsg |  5 +++++
 kernel/printk/printk.c             | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/ABI/testing/dev-kmsg b/Documentation/ABI/testing/dev-kmsg
index f307506eb54c..1e6c28b1942b 100644
--- a/Documentation/ABI/testing/dev-kmsg
+++ b/Documentation/ABI/testing/dev-kmsg
@@ -56,6 +56,11 @@ 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 ad4606234545..d1a219759ed3 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -963,6 +963,16 @@ 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.24.1

Re: [PATCH v2] kernel/printk: add kmsg SEEK_CUR handling

[Sergey Senozhatsky]

On (20/03/17 07:33), Bruno Meneguele wrote:
> Userspace libraries, e.g. glibc's dprintf(), perform a SEEK_CUR operation
> over any file descriptor requested to make sure the current position isn't
> pointing to junk due to previous manipulation of that same fd. And whenever
> that fd doesn't have support for such operation, the userspace code expects
> -ESPIPE to be returned.
> 
> However, when the fd in question references the /dev/kmsg interface, the
> current kernel code state returns -EINVAL instead, causing an unexpected
> behavior in userspace: in the case of glibc, when -ESPIPE is returned it
> gets ignored and the call completes successfully, while returning -EINVAL
> forces dprintf to fail without performing any action over that fd:
> 
>   if (_IO_SEEKOFF (fp, (off64_t)0, _IO_seek_cur, _IOS_INPUT|_IOS_OUTPUT) ==
>   _IO_pos_BAD && errno != ESPIPE)
>     return NULL;
> 
> With this patch we make sure to return the correct value when SEEK_CUR is
> requested over kmsg and also add some kernel doc information to formalize
> this behavior.
> 
> Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>

Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss