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: <509180f3-4cc1-4cc2-9d43-5a1e728fb718@oracle.com>
Date: Fri, 25 Oct 2024 15:04:08 +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 13:36, Ritesh Harjani (IBM) wrote:
>>> So user will anyway will have to be made aware of not to
>>> attempt writes of fashion which can cause them such penalties.
>>>
>>> As patch-6 mentions this is a base support for bs = ps systems for
>>> enabling atomic writes using bigalloc. For now we return -EINVAL when we
>>> can't allocate a continuous user requested mapping which means it won't
>>> support operations of types 8k followed by 16k.
>>>
>> That's my least-preferred option.
>>
>> I think better would be reject atomic writes that cover unwritten
>> extents always - but that boat is about to sail...
> That's what this patch does.

Not really.

Currently we have 2x iomap restrictions:
a. mapping length must equal fs block size
b. bio created must equal total write size

This patch just says that the mapping length must equal total write size 
(instead of a.). So quite similar to b.

> For whatever reason if we couldn't allocate
> a single contiguous region of requested size for atomic write, then we
> reject the request always, isn't it. Or maybe I didn't understand your comment.

As the simplest example, for an atomic write to an empty file, there 
should only be a single mapping returned to iomap_dio_bio_iter() and 
that would be of IOMAP_UNWRITTEN type. And we don't reject that.

> 
> If others prefer - we can maybe add such a check (e.g. ext4_dio_atomic_write_checks())
> for atomic writes in ext4_dio_write_checks(), similar to how we detect
> overwrites case to decide whether we need a read v/s write semaphore.
> So this can check if the user has a partially allocated extent for the
> user requested region and if yes, we can return -EINVAL from
> ext4_dio_write_iter() itself.
 > > I think this maybe better option than waiting until ->iomap_begin().
> This might also bring all atomic write constraints to be checked in one
> place i.e. during ext4_file_write_iter() itself.

Something like this can be done once we decide how atomic writing to 
regions which cover mixed unwritten and written extents is to be handled.

Thanks,
John


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ