linux-kernel - Re: [PATCH 5/6] iomap: Lift blocksize restriction on atomic writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1efb8d6d-ba2e-499d-abc5-e4f9a1e54e89@oracle.com>
Date: Fri, 25 Oct 2024 09:52:39 +0100
From: John Garry <john.g.garry@...cle.com>
To: "Ritesh Harjani (IBM)" <ritesh.list@...il.com>, linux-ext4@...r.kernel.org
Cc: Theodore Ts'o <tytso@....edu>, Jan Kara <jack@...e.cz>,
        "Darrick J . Wong" <djwong@...nel.org>,
        Christoph Hellwig
 <hch@...radead.org>,
        Ojaswin Mujoo <ojaswin@...ux.ibm.com>,
        Dave Chinner <david@...morbit.com>, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 5/6] iomap: Lift blocksize restriction on atomic writes

On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote:
> Filesystems like ext4 can submit writes in multiples of blocksizes.
> But we still can't allow the writes to be split. Hence let's check if
> the iomap_length() is same as iter->len or not.
> 
> This shouldn't affect XFS since it anyways checks for this in
> xfs_file_write_iter() to not support atomic write size request of more
> than FS blocksize.
> 
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>
> ---
>   fs/iomap/direct-io.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index ed4764e3b8f0..1d33b4239b3e 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -306,7 +306,7 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
>   	size_t copied = 0;
>   	size_t orig_count;
>   
> -	if (atomic && length != fs_block_size)
> +	if (atomic && length != iter->len)
>   		return -EINVAL;

Here you expect just one iter for an atomic write always.

In 6/6, you are saying that iomap does not allow an atomic write which 
covers unwritten and written extents, right?

But for writing a single fs block atomically, we don't mandate it to be 
in unwritten state. So there is a difference in behavior in writing a 
single FS block vs multiple FS blocks atomically.

So we have 3x choices, as I see:
a. add a check now in iomap that the extent is in written state (for an 
atomic write)
b. add extent zeroing code, as I was trying for originally
c. document this peculiarity

Thanks,
John