linux-kernel - Re: kmsg: lseek errors confuse glibc's dprintf

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: <20150123150939.f96d28a380fc5ec1f3894d3a@linux-foundation.org>
Date:	Fri, 23 Jan 2015 15:09:39 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Mike Crowe <mac@...owe.com>
Cc:	linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
	Kay Sievers <kay@...y.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: kmsg: lseek errors confuse glibc's dprintf

On Thu, 15 Jan 2015 17:31:32 +0000 Mike Crowe <mac@...owe.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
> 
> >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;

What's actually going on here?  What is the significance of
file->private_data==NULL and why does this code treat it as an error?

>  	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;

Can we actually do something useful here?  Return some value which can
be fed back into SEEK_SET to restore the file position?

>  	default:
>  		ret = -EINVAL;
>  	}

--
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/