linux-kernel - Re: [RFC PATCH] sysfs: refill attribute buffer when reading from offset 0

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: <20080405232959.c6504be8.akpm@linux-foundation.org>
Date:	Sat, 5 Apr 2008 23:29:59 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Dan Williams <dan.j.williams@...el.com>
Cc:	gregkh@...e.de, linux-kernel@...r.kernel.org, htejun@...il.com,
	neilb@...e.de
Subject: Re: [RFC PATCH] sysfs: refill attribute buffer when reading from
 offset 0

On Sat, 05 Apr 2008 11:41:22 -0700 Dan Williams <dan.j.williams@...el.com> wrote:

> Requiring userspace to close and re-open sysfs attributes has been the
> policy since before 2.6.12.  It allows userspace to get a consistent
> snapshot of kernel state and consume it with incremental reads and seeks.
> 
> Now, if the file position is zero the kernel assumes userspace wants to see
> the new value.

This does sound a sensible change.

>  The application for this change is to allow a userspace
> RAID metadata handler to check the state of an array without causing any
> memory allocations.  Thus not causing writeback to a raid array that might
> be blocked waiting for userspace to take action.

Although that sounds like a rather, umm, optimistic application.  I guess
if everything's mlocked you might get lucky.

> Cc: NeilBrown <neilb@...e.de>
> Cc: Tejun Heo <htejun@...il.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
> 
>  fs/sysfs/file.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index baa663e..0a26ba8 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -128,7 +128,7 @@ sysfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>  	ssize_t retval = 0;
>  
>  	mutex_lock(&buffer->mutex);
> -	if (buffer->needs_read_fill) {
> +	if (buffer->needs_read_fill || *ppos == 0) {
>  		retval = fill_read_buffer(file->f_path.dentry,buffer);
>  		if (retval)
>  			goto out;
> @@ -409,8 +409,7 @@ static int sysfs_release(struct inode *inode, struct file *filp)
>   * return POLLERR|POLLPRI, and select will return the fd whether
>   * it is waiting for read, write, or exceptions.
>   * Once poll/select indicates that the value has changed, you
> - * need to close and re-open the file, as simply seeking and reading
> - * again will not get new data, or reset the state of 'poll'.
> + * need to close and re-open the file, or seek to 0 and read again.
>   * Reminder: this only works for attributes which actively support
>   * it, and it is not possible to test an attribute from userspace
>   * to see if it supports poll (Neither 'poll' nor 'select' return
> 

Has this been tested with pread()?  That should work - doing an lseek+read
is plain dopey.

Can we now remove need_read_fill?  Not if we want to support
open+lseek+read, I guess - this initial read might not be at offset
zero.

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