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]
Date: Wed, 21 Feb 2024 09:00:31 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: John Garry <john.g.garry@...cle.com>
Cc: hch@....de, viro@...iv.linux.org.uk, brauner@...nel.org,
	dchinner@...hat.com, jack@...e.cz, chandan.babu@...cle.com,
	martin.petersen@...cle.com, linux-kernel@...r.kernel.org,
	linux-xfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	tytso@....edu, jbongio@...gle.com, ojaswin@...ux.ibm.com
Subject: Re: [PATCH 6/6] fs: xfs: Set FMODE_CAN_ATOMIC_WRITE for
 FS_XFLAG_ATOMICWRITES set

On Wed, Feb 14, 2024 at 12:36:40PM +0000, John Garry wrote:
> On 13/02/2024 17:59, Darrick J. Wong wrote:
> > > > Shouldn't we check that the device supports AWU at all before turning on
> > > > the FMODE flag?
> > > Can we easily get this sort of bdev info here?
> > > 
> > > Currently if we do try to issue an atomic write and AWU for the bdev is
> > > zero, then XFS iomap code will reject it.
> > Hmm.  Well, if we move towards pushing all the hardware checks out of
> > xfs/iomap and into whatever goes on underneath submit_bio then I guess
> > we don't need to check device support here at all.
> 
> Yeah, I have been thinking about this. But I was still planning on putting a
> "bdev on atomic write" check here, as you mentioned.
> 
> But is this a proper method to access the bdev for an xfs inode:
> 
> STATIC bool
> xfs_file_can_atomic_write(
> struct xfs_inode *inode)
> {
> 	struct xfs_buftarg *target = xfs_inode_buftarg(inode);
> 	struct block_device *bdev = target->bt_bdev;
> 
> 	if (!xfs_inode_atomicwrites(inode))
> 		return false;
> 
> 	return bdev_can_atomic_write(bdev);
> }

There's still a TOCTOU race problem if the bdev gets reconfigured
between xfs_file_can_atomic_write and submit_bio.

However, if you're only using this to advertise the capability via statx
then I suppose that's fine -- userspace has to have some means of
discovering the ability at all.  Userspace is also inherently racy.

> I do notice the dax check in xfs_bmbt_to_iomap() when assigning iomap->bdev,
> which is creating some doubt?

Do you mean this?

	if (mapping_flags & IOMAP_DAX)
		iomap->dax_dev = target->bt_daxdev;
	else
		iomap->bdev = target->bt_bdev;

The dax path wants dax_dev set so that it can do the glorified memcpy
operation, and it doesn't need (or want) a block device.

--D

> Thanks,
> John
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ