2021-01-20 20:55:40

by Christoph Hellwig

[permalink] [raw]
Subject: restore splice and sendfile support on kernfs

Hi Greg and Tejun,

this fixes a regression in Linux 5.10 that stopped sendfile and splice
from working on kernfs/sysfs files.

Diffstt:
file.c | 65 ++++++++++++++++++++++++-----------------------------------------
1 file changed, 24 insertions(+), 41 deletions(-)


2021-01-21 00:59:41

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/3] kernfs: implement ->read_iter

Switch kernfs to implement the read_iter method instead of plain old
read to prepare to supporting splice and sendfile again.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/kernfs/file.c | 35 ++++++++++++-----------------------
1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index f277d023ebcd14..8276e4c8722d8c 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -14,6 +14,7 @@
#include <linux/pagemap.h>
#include <linux/sched/mm.h>
#include <linux/fsnotify.h>
+#include <linux/uio.h>

#include "kernfs-internal.h"

@@ -180,11 +181,10 @@ static const struct seq_operations kernfs_seq_ops = {
* it difficult to use seq_file. Implement simplistic custom buffering for
* bin files.
*/
-static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
- char __user *user_buf, size_t count,
- loff_t *ppos)
+static ssize_t kernfs_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
{
- ssize_t len = min_t(size_t, count, PAGE_SIZE);
+ struct kernfs_open_file *of = kernfs_of(iocb->ki_filp);
+ ssize_t len = min_t(size_t, iov_iter_count(iter), PAGE_SIZE);
const struct kernfs_ops *ops;
char *buf;

@@ -210,7 +210,7 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
of->event = atomic_read(&of->kn->attr.open->event);
ops = kernfs_ops(of->kn);
if (ops->read)
- len = ops->read(of, buf, len, *ppos);
+ len = ops->read(of, buf, len, iocb->ki_pos);
else
len = -EINVAL;

@@ -220,12 +220,12 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
if (len < 0)
goto out_free;

- if (copy_to_user(user_buf, buf, len)) {
+ if (copy_to_iter(buf, len, iter) != len) {
len = -EFAULT;
goto out_free;
}

- *ppos += len;
+ iocb->ki_pos += len;

out_free:
if (buf == of->prealloc_buf)
@@ -235,22 +235,11 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of,
return len;
}

-/**
- * kernfs_fop_read - kernfs vfs read callback
- * @file: file pointer
- * @user_buf: data to write
- * @count: number of bytes
- * @ppos: starting offset
- */
-static ssize_t kernfs_fop_read(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
+static ssize_t kernfs_fop_read_iter(struct kiocb *iocb, struct iov_iter *iter)
{
- struct kernfs_open_file *of = kernfs_of(file);
-
- if (of->kn->flags & KERNFS_HAS_SEQ_SHOW)
- return seq_read(file, user_buf, count, ppos);
- else
- return kernfs_file_direct_read(of, user_buf, count, ppos);
+ if (kernfs_of(iocb->ki_filp)->kn->flags & KERNFS_HAS_SEQ_SHOW)
+ return seq_read_iter(iocb, iter);
+ return kernfs_file_read_iter(iocb, iter);
}

/**
@@ -960,7 +949,7 @@ void kernfs_notify(struct kernfs_node *kn)
EXPORT_SYMBOL_GPL(kernfs_notify);

const struct file_operations kernfs_file_fops = {
- .read = kernfs_fop_read,
+ .read_iter = kernfs_fop_read_iter,
.write = kernfs_fop_write,
.llseek = generic_file_llseek,
.mmap = kernfs_fop_mmap,
--
2.29.2

2021-01-21 01:18:33

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/3] kernfs: implement ->write_iter

Switch kernfs to implement the write_iter method instead of plain old
write to prepare to supporting splice and sendfile again.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/kernfs/file.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 8276e4c8722d8c..b1a5cccf189ec7 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -242,13 +242,7 @@ static ssize_t kernfs_fop_read_iter(struct kiocb *iocb, struct iov_iter *iter)
return kernfs_file_read_iter(iocb, iter);
}

-/**
- * kernfs_fop_write - kernfs vfs write callback
- * @file: file pointer
- * @user_buf: data to write
- * @count: number of bytes
- * @ppos: starting offset
- *
+/*
* Copy data in from userland and pass it to the matching kernfs write
* operation.
*
@@ -258,20 +252,18 @@ static ssize_t kernfs_fop_read_iter(struct kiocb *iocb, struct iov_iter *iter)
* modify only the the value you're changing, then write entire buffer
* back.
*/
-static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
- size_t count, loff_t *ppos)
+static ssize_t kernfs_fop_write_iter(struct kiocb *iocb, struct iov_iter *iter)
{
- struct kernfs_open_file *of = kernfs_of(file);
+ struct kernfs_open_file *of = kernfs_of(iocb->ki_filp);
+ ssize_t len = iov_iter_count(iter);
const struct kernfs_ops *ops;
- ssize_t len;
char *buf;

if (of->atomic_write_len) {
- len = count;
if (len > of->atomic_write_len)
return -E2BIG;
} else {
- len = min_t(size_t, count, PAGE_SIZE);
+ len = min_t(size_t, len, PAGE_SIZE);
}

buf = of->prealloc_buf;
@@ -282,7 +274,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
if (!buf)
return -ENOMEM;

- if (copy_from_user(buf, user_buf, len)) {
+ if (copy_from_iter(buf, len, iter) != len) {
len = -EFAULT;
goto out_free;
}
@@ -301,7 +293,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,

ops = kernfs_ops(of->kn);
if (ops->write)
- len = ops->write(of, buf, len, *ppos);
+ len = ops->write(of, buf, len, iocb->ki_pos);
else
len = -EINVAL;

@@ -309,7 +301,7 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf,
mutex_unlock(&of->mutex);

if (len > 0)
- *ppos += len;
+ iocb->ki_pos += len;

out_free:
if (buf == of->prealloc_buf)
@@ -662,7 +654,7 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)

/*
* Write path needs to atomic_write_len outside active reference.
- * Cache it in open_file. See kernfs_fop_write() for details.
+ * Cache it in open_file. See kernfs_fop_write_iter() for details.
*/
of->atomic_write_len = ops->atomic_write_len;

@@ -950,7 +942,7 @@ EXPORT_SYMBOL_GPL(kernfs_notify);

const struct file_operations kernfs_file_fops = {
.read_iter = kernfs_fop_read_iter,
- .write = kernfs_fop_write,
+ .write_iter = kernfs_fop_write_iter,
.llseek = generic_file_llseek,
.mmap = kernfs_fop_mmap,
.open = kernfs_fop_open,
--
2.29.2

2021-01-21 01:19:52

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 3/3] kernfs: wire up ->splice_read and ->splice_write

Wire up the splice_read and splice_write methods to the default
helpers using ->read_iter and ->write_iter now that those are
implemented for kernfs. This restores support to use splice and
sendfile on kernfs files.

Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
Reported-by: Siddharth Gupta <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Tested-by: Siddharth Gupta <[email protected]>
---
fs/kernfs/file.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index b1a5cccf189ec7..c7571931214751 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -949,6 +949,8 @@ const struct file_operations kernfs_file_fops = {
.release = kernfs_fop_release,
.poll = kernfs_fop_poll,
.fsync = noop_fsync,
+ .splice_read = generic_file_splice_read,
+ .splice_write = iter_file_splice_write,
};

/**
--
2.29.2

2021-01-21 17:33:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: restore splice and sendfile support on kernfs

On Wed, Jan 20, 2021 at 09:46:28PM +0100, Christoph Hellwig wrote:
> Hi Greg and Tejun,
>
> this fixes a regression in Linux 5.10 that stopped sendfile and splice
> from working on kernfs/sysfs files.
>
> Diffstt:
> file.c | 65 ++++++++++++++++++++++++-----------------------------------------
> 1 file changed, 24 insertions(+), 41 deletions(-)

Thanks for these, now queued up!

greg k-h

2021-01-21 17:45:14

by Robert Karszniewicz

[permalink] [raw]
Subject: Re: restore splice and sendfile support on kernfs

On 1/20/21 9:46 PM, Christoph Hellwig wrote:
> Hi Greg and Tejun,
>
> this fixes a regression in Linux 5.10 that stopped sendfile and splice
> from working on kernfs/sysfs files.
>
> Diffstt:
> file.c | 65 ++++++++++++++++++++++++-----------------------------------------
> 1 file changed, 24 insertions(+), 41 deletions(-)
>

Tested-by: Robert Karszniewicz <[email protected]>

Confirm that it fixes firmwared for me.

Also, I am 100% sure that the commit that actually breaks firmwared for me is
4d03e3cc5982 ("fs: don't allow kernel reads and writes without iter ops")
which is one commit right before
36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
. I just verified it again.

Thank you,
Robert