On Mon, Jan 10, 2022 at 09:48:27AM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > According to Dave lore, these ioctls originated in the early 1990s in > Irix EFS as a (somewhat clunky) way to preallocate space at the end of a > file. Heh. "Dave lore". Better reference - glibc Irix v4 compatibility header file from 1997: https://chromium.googlesource.com/chromiumos/third_party/glibc-ports/+/master/sysdeps/unix/sysv/irix4/bits/fcntl.h What it tells us is that fcntl(F_ALLOCSP) was supported on Irix 4.0 and the release dates for Irix 4 were 09/91 to 04/93. So there's the "for EFS in the early 1990s" date in a more concrete form... XFS was first released 18 months later in Irix 5.3 in December 1994.... > Irix XFS, naturally, picked up these ioctls to maintain > compatibility, which meant that they were ported to Linux in the early > 2000s. > > Recently it was pointed out to me they still lurk in the kernel, even > though the Linux fallocate syscall supplanted the functionality a long > time ago. fstests doesn't seem to include any real functional or stress > tests for these ioctls, which means that the code quality is ... very > questionable. Most notably, it was a stale disk block exposure vector > for 21 years and nobody noticed or complained. As mature programmers > say, "If you're not testing it, it's broken." > > Given all that, let's withdraw these ioctls from the XFS userspace API. > Normally we'd set a long deprecation process, but I estimate that there > aren't any real users, so let's trigger a warning in dmesg and return > -ENOTTY. *nod* > @@ -1965,13 +1884,10 @@ xfs_file_ioctl( > case XFS_IOC_ALLOCSP: > case XFS_IOC_FREESP: > case XFS_IOC_ALLOCSP64: > - case XFS_IOC_FREESP64: { > - xfs_flock64_t bf; > - > - if (copy_from_user(&bf, arg, sizeof(bf))) > - return -EFAULT; > - return xfs_ioc_space(filp, &bf); > - } > + case XFS_IOC_FREESP64: > + xfs_warn_once(mp, > + "dangerous XFS_IOC_{ALLOC,FREE}SP ioctls no longer supported"); > + return -ENOTTY; I wouldn't even say "dangerous", just that they are no longer supported. I would dump the process name, too, so we can identify what application (if any) is still using this, and maybe even append "Use fallocate(2) instead." Otherwise looks good. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
