lkml.org 
[lkml]   [2019]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: kmsg: lseek errors confuse glibc's dprintf
Hello Mike and all,

On Thu, 15 Jan 2015 17:31:32 +0000
Mike Crowe <mac@mcrowe.com> wrote:

> glibc's dprintf implementation does not work correctly with /dev/kmsg file
> descriptors because glibc treats receiving EBADF and EINVAL from lseek when
> trying to determine the current file position as errors. See
> https://sourceware.org/bugzilla/show_bug.cgi?id=17830

we need to conclude on this issue. This is a real bug which is ignored
for 4 years now. Mike, would you like to re-send a formal patch?
I can do it as well, preserving a link to your original patch/report.
In case you'd like to post it yourself, I can be a tester and/or
provide a reproducer.

> >>From what I can tell prior to Kay Sievers printk record commit
> e11fea92e13fb91c50bacca799a6131c81929986, calling lseek(fd, 0, SEEK_CUR)
> with such a file descriptor would not return an error.
>
> Prior to Kay's change, Arnd Bergmann's commit
> 6038f373a3dc1f1c26496e60b6c40b164716f07e seemed to go to some lengths to
> preserve the successful return code rather than returning (the perhaps more
> logical) -ESPIPE.
>
> glibc is happy with either a successful return or -ESPIPE.
>
> For maximum compatibility it seems that success should be returned but
> given Kay's new seek interface perhaps this isn't helpful.
>
> This patch ensures that such a seek succeeds:
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 02d6b6d..b3ff6f0 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -693,7 +693,7 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
> loff_t ret = 0;
>
> if (!user)
> - return -EBADF;
> + return (whence == SEEK_CUR) ? 0 : -EBADF;
> if (offset)
> return -ESPIPE;
>
> @@ -718,6 +718,11 @@ 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:
> + /* For compatibility with userspace requesting the
> + * current file position. */
> + ret = 0;
> + break;
> default:
> ret = -EINVAL;
> }
>
> (although it could be argued that the !user case should return -ESPIPE
> rather than EBADF since the file descriptor _is_ valid.)
>
> and this patch causes a failure that glibc is prepared to accept:
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 02d6b6d..f6b0c93 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -693,7 +693,7 @@ static loff_t devkmsg_llseek(struct file *file, loff_t offset, int whence)
> loff_t ret = 0;
>
> if (!user)
> - return -EBADF;
> + return -ESPIPE;
> if (offset)
> return -ESPIPE;
>
> @@ -718,6 +718,11 @@ 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:
> + /* For compatibility with userspace expecting SEEK_CUR
> + * to not yield EINVAL. */
> + ret = -ESPIPE;
> + break;
> default:
> ret = -EINVAL;
> }
>
> Either makes dprintf work, but is either the right solution?
>
> Thanks.
>
> Mike.


--
Alexander Sverdlin.

\
 
 \ /
  Last update: 2019-03-21 10:47    [W:0.094 / U:10.584 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site