[PATCH driver-core-next] sysfs: use generic_file_llseek() for sysfs_file_operations

[PATCH driver-core-next] sysfs: use generic_file_llseek() for sysfs_file_operations

[Tejun Heo]

Hey, Greg.

Here's proper patch with description and SOB.  I'll be traveling from
tomorrow so I might not be responsive for some days.  Can you please
apply it once Heiko confirms it fixes the issue?

Thanks!

------- 8< -------
13c589d5b0ac6 ("sysfs: use seq_file when reading regular files")
converted regular sysfs files to use seq_file.  The commit substituted
generic_file_llseek() with seq_lseek() for llseek implementation.

Before the change, all regular sysfs files were allowed to seek to any
position in [0, PAGE_SIZE] as the file size is always PAGE_SIZE and
generic_file_llseek() allows any seeking inside the range under file
size; however, seq_lseek()'s behavior is different.  It traverses the
output by repeatedly invoking ->show() until it reaches the target
offset or traversal indicates EOF.  As seq_files are fully dynamic and
may not end at all, it doesn't support seeking from the end
(SEEK_END).

Apparently, there are userland tools which uses SEEK_END to discover
the buffer size to use and the switch to seq_lseek() disturbs them as
SEEK_END fails with -EINVAL.

The only benefits of using seq_lseek() instead of
generic_file_llseek() are

* Early failure.  If traversing to certain file position should fail,
  seq_lseek() will report such failures on lseek(2) instead of the
  following read/write operations.

* EOF detection.  While SEEK_END is not supported, SEEK_SET/CUR +
  large offset can be used to detect eof - eof at the time of the seek
  anyway as the file size may change dynamically.

Both aren't necessary for sysfs or prospect kernfs users.  Revert to
genefic_file_llseek() and preserve the original behavior.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Link: https://lkml.kernel.org/r/20131031114358.GA5551@osiris
---
 fs/sysfs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -800,7 +800,7 @@ EXPORT_SYMBOL_GPL(sysfs_notify);
 const struct file_operations sysfs_file_operations = {
 	.read		= seq_read,
 	.write		= sysfs_write_file,
-	.llseek		= seq_lseek,
+	.llseek		= generic_file_llseek,
 	.open		= sysfs_open_file,
 	.release	= sysfs_release,
 	.poll		= sysfs_poll,

Re: [PATCH driver-core-next] sysfs: use generic_file_llseek() for sysfs_file_operations

[Greg Kroah-Hartman]

On Fri, Nov 01, 2013 at 01:16:53PM -0400, Tejun Heo wrote:
> Hey, Greg.
> 
> Here's proper patch with description and SOB.  I'll be traveling from
> tomorrow so I might not be responsive for some days.  Can you please
> apply it once Heiko confirms it fixes the issue?

Will do.

Heiko, let me know if this fixes your problem or not, thanks.

greg k-h

Re: [PATCH driver-core-next] sysfs: use generic_file_llseek() for sysfs_file_operations

[Heiko Carstens]

On Fri, Nov 01, 2013 at 10:19:13AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Nov 01, 2013 at 01:16:53PM -0400, Tejun Heo wrote:
> > Hey, Greg.
> > 
> > Here's proper patch with description and SOB.  I'll be traveling from
> > tomorrow so I might not be responsive for some days.  Can you please
> > apply it once Heiko confirms it fixes the issue?
> 
> Will do.
> 
> Heiko, let me know if this fixes your problem or not, thanks.

Yes it does fix the problem. strace output looks like before:

[pid  2495] open("/sys/class/net/eth0/broadcast", O_RDONLY) = 5
[pid  2495] lseek(5, 0, SEEK_END)       = 4096
[pid  2495] lseek(5, 0, SEEK_SET)       = 0
[pid  2495] read(5, "ff:ff:ff:ff:ff:ff\n", 4096) = 18
[pid  2495] close(5)                    = 0

Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>