[PATCH] fs/iomap: Fix write path page prefaulting

[PATCH] fs/iomap: Fix write path page prefaulting

[Andreas Gruenbacher]

When part of the user buffer passed to generic_perform_write() or
iomap_file_buffered_write() cannot be faulted in for reading, the entire
write currently fails.

The correct behavior would be to write all the data that can be written,
up to the point of failure.  Since commit a6294593e8a1 ("iov_iter: Turn
iov_iter_fault_in_readable into fault_in_iov_iter_readable"), we have
enough information to implement that, so change the code to do that.

We already take into account that pages faulted in may no longer be
resident by the time they are accessed, so the code will also behave
correctly when part of the buffer isn't faulted in in the first place.

This leads to an intentional user-visible change when the buffer passed
to write calls contains unmapped or poisoned pages.

(This change obsoletes commit 554c577cee95 ("gfs2: Prevent endless loops
in gfs2_file_buffered_write").)

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/iomap/buffered-io.c | 2 +-
 mm/filemap.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 1753c26c8e76..54516ab464cd 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -750,7 +750,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
 		 * same page as we're writing to, without it being marked
 		 * up-to-date.
 		 */
-		if (unlikely(fault_in_iov_iter_readable(i, bytes))) {
+		if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
 			status = -EFAULT;
 			break;
 		}
diff --git a/mm/filemap.c b/mm/filemap.c
index daa0e23a6ee6..767202265048 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3743,7 +3743,7 @@ ssize_t generic_perform_write(struct file *file,
 		 * same page as we're writing to, without it being marked
 		 * up-to-date.
 		 */
-		if (unlikely(fault_in_iov_iter_readable(i, bytes))) {
+		if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) {
 			status = -EFAULT;
 			break;
 		}
-- 
2.31.1

Re: [PATCH] fs/iomap: Fix write path page prefaulting

[Catalin Marinas]

On Tue, Nov 23, 2021 at 04:18:12PM +0100, Andreas Gruenbacher wrote:
> When part of the user buffer passed to generic_perform_write() or
> iomap_file_buffered_write() cannot be faulted in for reading, the entire
> write currently fails.
> 
> The correct behavior would be to write all the data that can be written,
> up to the point of failure.  Since commit a6294593e8a1 ("iov_iter: Turn
> iov_iter_fault_in_readable into fault_in_iov_iter_readable"), we have
> enough information to implement that, so change the code to do that.
> 
> We already take into account that pages faulted in may no longer be
> resident by the time they are accessed, so the code will also behave
> correctly when part of the buffer isn't faulted in in the first place.
> 
> This leads to an intentional user-visible change when the buffer passed
> to write calls contains unmapped or poisoned pages.
> 
> (This change obsoletes commit 554c577cee95 ("gfs2: Prevent endless loops
> in gfs2_file_buffered_write").)
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

FWIW:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

This would be more consistent as in my tests on a vanilla kernel ext4
and btrfs fail with EFAULT while gfs2 copies as much as it can before
hitting the fault.

-- 
Catalin

Re: [PATCH] fs/iomap: Fix write path page prefaulting

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>