overlayfs using jffs2 as the upper filesystem would fail in some cases
since moving to v5.10. The test case used was to run 'touch' on a file
that exists in the lower fs, causing the modification time to be
updated. It returns EINVAL when the bug is triggered.
A bisection showed this was introduced in v5.9-rc1, with commit
36e2c7421f02 ("fs: don't allow splice read/write without explicit ops").
Reverting that commit restores the expected behaviour.
Some digging showed that this was due to jffs2 lacking an implementation
of splice_write. (For unknown reasons the warn_unsupported that should
trigger was not displaying any output).
Adding this patch resolved the issue and the test now passes.
Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
Signed-off-by: Joel Stanley <[email protected]>
---
fs/jffs2/file.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index f8fb89b10227..4fc8cd698d1a 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations =
.mmap = generic_file_readonly_mmap,
.fsync = jffs2_fsync,
.splice_read = generic_file_splice_read,
+ .splice_write = iter_file_splice_write,
};
/* jffs2_file_inode_operations */
--
2.30.2
On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote:
> overlayfs using jffs2 as the upper filesystem would fail in some cases
> since moving to v5.10. The test case used was to run 'touch' on a file
> that exists in the lower fs, causing the modification time to be
> updated. It returns EINVAL when the bug is triggered.
>
> A bisection showed this was introduced in v5.9-rc1, with commit
> 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops").
> Reverting that commit restores the expected behaviour.
>
> Some digging showed that this was due to jffs2 lacking an implementation
> of splice_write. (For unknown reasons the warn_unsupported that should
> trigger was not displaying any output).
>
> Adding this patch resolved the issue and the test now passes.
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Tue, Mar 30, 2021 at 06:17:15PM +0200, Christoph Hellwig wrote:
> On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote:
> > overlayfs using jffs2 as the upper filesystem would fail in some cases
> > since moving to v5.10. The test case used was to run 'touch' on a file
> > that exists in the lower fs, causing the modification time to be
> > updated. It returns EINVAL when the bug is triggered.
> >
> > A bisection showed this was introduced in v5.9-rc1, with commit
> > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops").
> > Reverting that commit restores the expected behaviour.
> >
> > Some digging showed that this was due to jffs2 lacking an implementation
> > of splice_write. (For unknown reasons the warn_unsupported that should
> > trigger was not displaying any output).
> >
> > Adding this patch resolved the issue and the test now passes.
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
The same goes for quite a few other filesystems, actually - at least
adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs
and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs
might be too.
Christoph, do you see any problems with doing the same thing for that
bunch as well?
On Tue, Mar 30, 2021 at 06:31:00PM +0000, Al Viro wrote:
> On Tue, Mar 30, 2021 at 06:17:15PM +0200, Christoph Hellwig wrote:
> > On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote:
> > > overlayfs using jffs2 as the upper filesystem would fail in some cases
> > > since moving to v5.10. The test case used was to run 'touch' on a file
> > > that exists in the lower fs, causing the modification time to be
> > > updated. It returns EINVAL when the bug is triggered.
> > >
> > > A bisection showed this was introduced in v5.9-rc1, with commit
> > > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops").
> > > Reverting that commit restores the expected behaviour.
> > >
> > > Some digging showed that this was due to jffs2 lacking an implementation
> > > of splice_write. (For unknown reasons the warn_unsupported that should
> > > trigger was not displaying any output).
> > >
> > > Adding this patch resolved the issue and the test now passes.
> >
> > Looks good:
> >
> > Reviewed-by: Christoph Hellwig <[email protected]>
>
> The same goes for quite a few other filesystems, actually - at least
> adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs
> and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs
> might be too.
>
> Christoph, do you see any problems with doing the same thing for that
> bunch as well?
coda and udf as well, by the look of it...
On Tue, Mar 30, 2021 at 9:45 PM Joel Stanley <[email protected]> wrote:
>
> overlayfs using jffs2 as the upper filesystem would fail in some cases
> since moving to v5.10. The test case used was to run 'touch' on a file
> that exists in the lower fs, causing the modification time to be
> updated. It returns EINVAL when the bug is triggered.
>
> A bisection showed this was introduced in v5.9-rc1, with commit
> 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops").
> Reverting that commit restores the expected behaviour.
>
> Some digging showed that this was due to jffs2 lacking an implementation
> of splice_write. (For unknown reasons the warn_unsupported that should
> trigger was not displaying any output).
>
> Adding this patch resolved the issue and the test now passes.
>
> Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops")
> Signed-off-by: Joel Stanley <[email protected]>
> ---
> fs/jffs2/file.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
> index f8fb89b10227..4fc8cd698d1a 100644
> --- a/fs/jffs2/file.c
> +++ b/fs/jffs2/file.c
> @@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations =
> .mmap = generic_file_readonly_mmap,
> .fsync = jffs2_fsync,
> .splice_read = generic_file_splice_read,
> + .splice_write = iter_file_splice_write,
> };
>
> /* jffs2_file_inode_operations */
> --
> 2.30.2
>
Verified on g220a openbmc system that the patch fixes the issue.
Tested-by: Lei YU <[email protected]>
On Tue, Mar 30, 2021 at 06:31:00PM +0000, Al Viro wrote:
> The same goes for quite a few other filesystems, actually - at least
> adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs
> and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs
> might be too.
>
> Christoph, do you see any problems with doing the same thing for that
> bunch as well?
Linus asked to only fix things up where we get reports, but I think his
intent there was more random procfs and debugfs files rather than file
systems. So just doing the sweep should be fine.