linux-kernel - Re: [PATCH v7 09/14] xfs: add large atomic writes checks in xfs_direct_write_iomap

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: <cf7e2a1c-0b2c-4295-81e8-0f407fe72768@oracle.com>
Date: Tue, 15 Apr 2025 18:46:38 +0100
From: John Garry <john.g.garry@...cle.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: brauner@...nel.org, hch@....de, viro@...iv.linux.org.uk, jack@...e.cz,
        cem@...nel.org, linux-fsdevel@...r.kernel.org, dchinner@...hat.com,
        linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        ojaswin@...ux.ibm.com, ritesh.list@...il.com,
        martin.petersen@...cle.com, linux-ext4@...r.kernel.org,
        linux-block@...r.kernel.org, catherine.hoang@...cle.com,
        linux-api@...r.kernel.org
Subject: Re: [PATCH v7 09/14] xfs: add large atomic writes checks in
 xfs_direct_write_iomap_begin()

On 15/04/2025 18:34, Darrick J. Wong wrote:
>> +	/*
>> +	 * Spanning multiple extents would mean that multiple BIOs would be
>> +	 * issued, and so would lose atomicity required for REQ_ATOMIC-based
>> +	 * atomics.
>> +	 */
>> +	if (!imap_spans_range(imap, offset_fsb, end_fsb))
>> +		return false;
>> +
>> +	/*
>> +	 * The ->iomap_begin caller should ensure this, but check anyway.
>> +	 */
>> +	if (len > xfs_inode_buftarg(ip)->bt_bdev_awu_max)
>> +		return false;
> This needs to check len against bt_bdev_awu_min so that we don't submit
> too-short atomic writes to the hardware. 

Right, let me check this.

I think that we should only support sane HW which can write 1x FS block 
or more.

> Let's say that the hardware
> minimum is 32k and the fsblock size is 4k.  XFS can perform an out of
> place write for 4k-16k writes, but right now we'll just throw invalid
> commands at the bdev, and it'll return EINVAL.
> 
> /me wonders if statx should grow a atomic_write_unit_min_opt field
> too, unless everyone in block layer land is convinced that awu_min will
> always match lbasize?  (I probably missed that conversation)

Nothing states that it should (match lbasize), but again HW which can 
only write >1 FS block is something which I don't want to support (yet).

Thanks,
John