Re: [PATCH 2/3] xfs: disallow broken ioctls without compat-32-bit-time

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Tue, Jan 07, 2020 at 06:16:34AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 02, 2020 at 10:07:49AM -0800, Darrick J. Wong wrote:
> > > Sorry I missed that comment earlier. I've had a fresh look now, but
> > > I think we still need to deprecate XFS_IOC_SWAPEXT and add a
> > > v5 version of it, since the comparison will fail as soon as the range
> > > of the inode timestamps is extended beyond 2038, otherwise the
> > > comparison will always be false, or require comparing the truncated
> > > time values which would add yet another representation.
> > 
> > I prefer we replace the old SWAPEXT with a new version to get rid of
> > struct xfs_bstat.  Though a SWAPEXT_V5 probably only needs to contain
> > the *stat fields that swapext actually needs to check that the file
> > hasn't been changed, which would be ino/gen/btime/ctime.
> > 
> > (Maybe I'd add an offset/length too...)
> 
> And most importantly we need to lift it to the VFS instead of all the
> crazy fs specific interfaces at the moment.

Yeah.  Fixing that (and maybe adding an ioctl to set the FS UUID online)
were on my list for 5.6 but clearly I have to defer everything until 5.7
because we've just run out of time.

Uh... I started looking into unifying the ext4 and xfs defrag ioctl, but
gagged when I realized that the ext4 ioctl also handles the data copy
inside the kernel.  I think that's the sort of behavior we should /not/
allow into the new ioctl, though that also means that the required
changes for ext4/e4defrag will be non-trivial.

The btrfs defrag ioctl also contains thresholding information and
optional knobs to enable compression, which makes me wonder if we should
focus narrowly on swapext being "swap these extents but only if the
source file hasn't changed" and not necessarily defrag?

...in which case I wonder, can people (ab)use this interface for atomic
file updates?  Create an O_TMPFILE, reflink the source file into the
temp file, make your updates to the tempfile, and then swapext the
donor's file data back into the source file, but only if the source file
hasn't changed?

--D