MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix ext2_dio_write_iter() to propagate the error returned by
generic_write_sync() instead of silently discarding it, which could
cause write(2) to return success to userspace on O_SYNC/O_DSYNC files
even when the sync failed.

The correct pattern, already used in ext2_dax_write_iter() in the same
file and in ext4, xfs, f2fs among others, is:
    if (ret > 0)
        ret = generic_write_sync(iocb, ret);

Found by Linux Verification Center (linuxtesting.org) with SVACE.
     
Fixes: fb5de4358e1a ("ext2: Move direct-io to use iomap")
Signed-off-by: Danila Chernetsov <listdansp@mail.ru>
---
 fs/ext2/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index d9b1eb34694a..855a62e96c38 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -272,7 +272,7 @@ static ssize_t ext2_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 						 pos >> PAGE_SHIFT,
 						 endbyte >> PAGE_SHIFT);
 		if (ret > 0)
-			generic_write_sync(iocb, ret);
+			ret = generic_write_sync(iocb, ret);
 	}
 
 out_unlock:
-- 
2.25.1

On Sat 30-05-26 12:23:11, Danila Chernetsov wrote:
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Fix ext2_dio_write_iter() to propagate the error returned by
> generic_write_sync() instead of silently discarding it, which could
> cause write(2) to return success to userspace on O_SYNC/O_DSYNC files
> even when the sync failed.
> 
> The correct pattern, already used in ext2_dax_write_iter() in the same
> file and in ext4, xfs, f2fs among others, is:
>     if (ret > 0)
>         ret = generic_write_sync(iocb, ret);
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>      
> Fixes: fb5de4358e1a ("ext2: Move direct-io to use iomap")
> Signed-off-by: Danila Chernetsov <listdansp@mail.ru>

Thanks for the patch. I think it makes sense although I also think we
should also reflect the return value of filemap_write_and_wait_range()
(when it fails, return the error from ext2_dio_write_iter()) to be
consistent with handling errors during flushing of direct IO fallback. I'll
update the patch on commit.

								Honza

> ---
>  fs/ext2/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index d9b1eb34694a..855a62e96c38 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -272,7 +272,7 @@ static ssize_t ext2_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  						 pos >> PAGE_SHIFT,
>  						 endbyte >> PAGE_SHIFT);
>  		if (ret > 0)
> -			generic_write_sync(iocb, ret);
> +			ret = generic_write_sync(iocb, ret);
>  	}
>  
>  out_unlock:
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR