* [PATCHSET v2 0/3] xfs: kill dead ioctls for 5.17
@ 2022-01-11 23:22 Darrick J. Wong
2022-01-11 23:22 ` [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls Darrick J. Wong
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Darrick J. Wong @ 2022-01-11 23:22 UTC (permalink / raw)
To: djwong; +Cc: Dave Chinner, Eric Sandeen, linux-xfs
Hi all,
Let's retire some old ioctls that we don't want to support anymore!
v2: target FSSETDM too, and various tweaks to the ALLOCSP log messages
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=xfs-5.17-kill-ioctls
---
fs/xfs/libxfs/xfs_fs.h | 37 ++++-------------
fs/xfs/xfs_bmap_util.c | 7 +--
fs/xfs/xfs_bmap_util.h | 2 -
fs/xfs/xfs_file.c | 3 -
fs/xfs/xfs_ioctl.c | 102 +++++++-----------------------------------------
fs/xfs/xfs_ioctl.h | 6 ---
fs/xfs/xfs_ioctl32.c | 27 -------------
fs/xfs/xfs_ioctl32.h | 4 --
8 files changed, 27 insertions(+), 161 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls
2022-01-11 23:22 [PATCHSET v2 0/3] xfs: kill dead ioctls for 5.17 Darrick J. Wong
@ 2022-01-11 23:22 ` Darrick J. Wong
2022-01-13 3:47 ` Dave Chinner
2022-01-11 23:22 ` [PATCH 2/3] xfs: remove the XFS_IOC_{ALLOC,FREE}SP* definitions Darrick J. Wong
2022-01-11 23:22 ` [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions Darrick J. Wong
2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2022-01-11 23:22 UTC (permalink / raw)
To: djwong; +Cc: Eric Sandeen, linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
According to the glibc compat header for Irix 4, these ioctls originated
in April 1991 as a (somewhat clunky) way to preallocate space at the end
of a file on an EFS filesystem. XFS, which was released in Irix 5.3 in
December 1993, picked up these ioctls to maintain compatibility and 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.
See: CVE-2021-4155
Augments: 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/xfs_bmap_util.c | 7 ++--
fs/xfs/xfs_bmap_util.h | 2 +
fs/xfs/xfs_file.c | 3 +-
fs/xfs/xfs_ioctl.c | 93 +++---------------------------------------------
fs/xfs/xfs_ioctl.h | 6 ---
fs/xfs/xfs_ioctl32.c | 27 --------------
6 files changed, 10 insertions(+), 128 deletions(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 73a36b7be3bd..575060a7c768 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -771,8 +771,7 @@ int
xfs_alloc_file_space(
struct xfs_inode *ip,
xfs_off_t offset,
- xfs_off_t len,
- int alloc_type)
+ xfs_off_t len)
{
xfs_mount_t *mp = ip->i_mount;
xfs_off_t count;
@@ -865,8 +864,8 @@ xfs_alloc_file_space(
goto error;
error = xfs_bmapi_write(tp, ip, startoffset_fsb,
- allocatesize_fsb, alloc_type, 0, imapp,
- &nimaps);
+ allocatesize_fsb, XFS_BMAPI_PREALLOC, 0, imapp,
+ &nimaps);
if (error)
goto error;
diff --git a/fs/xfs/xfs_bmap_util.h b/fs/xfs/xfs_bmap_util.h
index 9f993168b55b..24b37d211f1d 100644
--- a/fs/xfs/xfs_bmap_util.h
+++ b/fs/xfs/xfs_bmap_util.h
@@ -54,7 +54,7 @@ int xfs_bmap_last_extent(struct xfs_trans *tp, struct xfs_inode *ip,
/* preallocation and hole punch interface */
int xfs_alloc_file_space(struct xfs_inode *ip, xfs_off_t offset,
- xfs_off_t len, int alloc_type);
+ xfs_off_t len);
int xfs_free_file_space(struct xfs_inode *ip, xfs_off_t offset,
xfs_off_t len);
int xfs_collapse_file_space(struct xfs_inode *, xfs_off_t offset,
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 27594738b0d1..d81a28cada35 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1052,8 +1052,7 @@ xfs_file_fallocate(
}
if (!xfs_is_always_cow_inode(ip)) {
- error = xfs_alloc_file_space(ip, offset, len,
- XFS_BMAPI_PREALLOC);
+ error = xfs_alloc_file_space(ip, offset, len);
if (error)
goto out_unlock;
}
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 8ea47a9d5aad..64a7ef4a7298 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -627,87 +627,6 @@ xfs_attrmulti_by_handle(
return error;
}
-int
-xfs_ioc_space(
- struct file *filp,
- xfs_flock64_t *bf)
-{
- struct inode *inode = file_inode(filp);
- struct xfs_inode *ip = XFS_I(inode);
- struct iattr iattr;
- enum xfs_prealloc_flags flags = XFS_PREALLOC_CLEAR;
- uint iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
- int error;
-
- if (inode->i_flags & (S_IMMUTABLE|S_APPEND))
- return -EPERM;
-
- if (!(filp->f_mode & FMODE_WRITE))
- return -EBADF;
-
- if (!S_ISREG(inode->i_mode))
- return -EINVAL;
-
- if (xfs_is_always_cow_inode(ip))
- return -EOPNOTSUPP;
-
- if (filp->f_flags & O_DSYNC)
- flags |= XFS_PREALLOC_SYNC;
- if (filp->f_mode & FMODE_NOCMTIME)
- flags |= XFS_PREALLOC_INVISIBLE;
-
- error = mnt_want_write_file(filp);
- if (error)
- return error;
-
- xfs_ilock(ip, iolock);
- error = xfs_break_layouts(inode, &iolock, BREAK_UNMAP);
- if (error)
- goto out_unlock;
- inode_dio_wait(inode);
-
- switch (bf->l_whence) {
- case 0: /*SEEK_SET*/
- break;
- case 1: /*SEEK_CUR*/
- bf->l_start += filp->f_pos;
- break;
- case 2: /*SEEK_END*/
- bf->l_start += XFS_ISIZE(ip);
- break;
- default:
- error = -EINVAL;
- goto out_unlock;
- }
-
- if (bf->l_start < 0 || bf->l_start > inode->i_sb->s_maxbytes) {
- error = -EINVAL;
- goto out_unlock;
- }
-
- if (bf->l_start > XFS_ISIZE(ip)) {
- error = xfs_alloc_file_space(ip, XFS_ISIZE(ip),
- bf->l_start - XFS_ISIZE(ip),
- XFS_BMAPI_PREALLOC);
- if (error)
- goto out_unlock;
- }
-
- iattr.ia_valid = ATTR_SIZE;
- iattr.ia_size = bf->l_start;
- error = xfs_vn_setattr_size(file_mnt_user_ns(filp), file_dentry(filp),
- &iattr);
- if (error)
- goto out_unlock;
-
- error = xfs_update_prealloc_flags(ip, flags);
-
-out_unlock:
- xfs_iunlock(ip, iolock);
- mnt_drop_write_file(filp);
- return error;
-}
-
/* Return 0 on success or positive error */
int
xfs_fsbulkstat_one_fmt(
@@ -1965,13 +1884,11 @@ 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,
+ "%s should use fallocate; XFS_IOC_{ALLOC,FREE}SP ioctl unsupported",
+ current->comm);
+ return -ENOTTY;
case XFS_IOC_DIOINFO: {
struct xfs_buftarg *target = xfs_inode_buftarg(ip);
struct dioattr da;
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index 845d3bcab74b..d4abba2c13c1 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -10,12 +10,6 @@ struct xfs_bstat;
struct xfs_ibulk;
struct xfs_inogrp;
-
-extern int
-xfs_ioc_space(
- struct file *filp,
- xfs_flock64_t *bf);
-
int
xfs_ioc_swapext(
xfs_swapext_t *sxp);
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 8783af203cfc..004ed2a251e8 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -27,22 +27,6 @@
_IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(type))
#ifdef BROKEN_X86_ALIGNMENT
-STATIC int
-xfs_compat_flock64_copyin(
- xfs_flock64_t *bf,
- compat_xfs_flock64_t __user *arg32)
-{
- if (get_user(bf->l_type, &arg32->l_type) ||
- get_user(bf->l_whence, &arg32->l_whence) ||
- get_user(bf->l_start, &arg32->l_start) ||
- get_user(bf->l_len, &arg32->l_len) ||
- get_user(bf->l_sysid, &arg32->l_sysid) ||
- get_user(bf->l_pid, &arg32->l_pid) ||
- copy_from_user(bf->l_pad, &arg32->l_pad, 4*sizeof(u32)))
- return -EFAULT;
- return 0;
-}
-
STATIC int
xfs_compat_ioc_fsgeometry_v1(
struct xfs_mount *mp,
@@ -445,17 +429,6 @@ xfs_file_compat_ioctl(
switch (cmd) {
#if defined(BROKEN_X86_ALIGNMENT)
- case XFS_IOC_ALLOCSP_32:
- case XFS_IOC_FREESP_32:
- case XFS_IOC_ALLOCSP64_32:
- case XFS_IOC_FREESP64_32: {
- struct xfs_flock64 bf;
-
- if (xfs_compat_flock64_copyin(&bf, arg))
- return -EFAULT;
- cmd = _NATIVE_IOC(cmd, struct xfs_flock64);
- return xfs_ioc_space(filp, &bf);
- }
case XFS_IOC_FSGEOMETRY_V1_32:
return xfs_compat_ioc_fsgeometry_v1(ip->i_mount, arg);
case XFS_IOC_FSGROWFSDATA_32: {
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] xfs: remove the XFS_IOC_{ALLOC,FREE}SP* definitions
2022-01-11 23:22 [PATCHSET v2 0/3] xfs: kill dead ioctls for 5.17 Darrick J. Wong
2022-01-11 23:22 ` [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls Darrick J. Wong
@ 2022-01-11 23:22 ` Darrick J. Wong
2022-01-11 23:22 ` [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions Darrick J. Wong
2 siblings, 0 replies; 7+ messages in thread
From: Darrick J. Wong @ 2022-01-11 23:22 UTC (permalink / raw)
To: djwong; +Cc: Dave Chinner, Eric Sandeen, linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
Now that we've made these ioctls defunct, move them from xfs_fs.h to
xfs_ioctl.c, which effectively removes them from the publicly supported
ioctl interfaces for XFS.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
---
fs/xfs/libxfs/xfs_fs.h | 8 ++++----
fs/xfs/xfs_ioctl.c | 9 +++++++++
fs/xfs/xfs_ioctl32.h | 4 ----
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index c43877c8a279..49c0e583d6bb 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -781,13 +781,13 @@ struct xfs_scrub_metadata {
* For 'documentation' purposed more than anything else,
* the "cmd #" field reflects the IRIX fcntl number.
*/
-#define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
-#define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
+/* XFS_IOC_ALLOCSP ------- deprecated 10 */
+/* XFS_IOC_FREESP -------- deprecated 11 */
#define XFS_IOC_DIOINFO _IOR ('X', 30, struct dioattr)
#define XFS_IOC_FSGETXATTR FS_IOC_FSGETXATTR
#define XFS_IOC_FSSETXATTR FS_IOC_FSSETXATTR
-#define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
-#define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
+/* XFS_IOC_ALLOCSP64 ----- deprecated 36 */
+/* XFS_IOC_FREESP64 ------ deprecated 37 */
#define XFS_IOC_GETBMAP _IOWR('X', 38, struct getbmap)
#define XFS_IOC_FSSETDM _IOW ('X', 39, struct fsdmidata)
#define XFS_IOC_RESVSP _IOW ('X', 40, struct xfs_flock64)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 64a7ef4a7298..03a6198c97f6 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1854,6 +1854,15 @@ xfs_fs_eofblocks_from_user(
return 0;
}
+/*
+ * These long-unused ioctls were removed from the official ioctl API in 5.17,
+ * but retain these definitions so that we can log warnings about them.
+ */
+#define XFS_IOC_ALLOCSP _IOW ('X', 10, struct xfs_flock64)
+#define XFS_IOC_FREESP _IOW ('X', 11, struct xfs_flock64)
+#define XFS_IOC_ALLOCSP64 _IOW ('X', 36, struct xfs_flock64)
+#define XFS_IOC_FREESP64 _IOW ('X', 37, struct xfs_flock64)
+
/*
* Note: some of the ioctl's return positive numbers as a
* byte count indicating success, such as readlink_by_handle.
diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h
index 9929482bf358..fc5a91f3a5e0 100644
--- a/fs/xfs/xfs_ioctl32.h
+++ b/fs/xfs/xfs_ioctl32.h
@@ -154,10 +154,6 @@ typedef struct compat_xfs_flock64 {
__s32 l_pad[4]; /* reserve area */
} compat_xfs_flock64_t;
-#define XFS_IOC_ALLOCSP_32 _IOW('X', 10, struct compat_xfs_flock64)
-#define XFS_IOC_FREESP_32 _IOW('X', 11, struct compat_xfs_flock64)
-#define XFS_IOC_ALLOCSP64_32 _IOW('X', 36, struct compat_xfs_flock64)
-#define XFS_IOC_FREESP64_32 _IOW('X', 37, struct compat_xfs_flock64)
#define XFS_IOC_RESVSP_32 _IOW('X', 40, struct compat_xfs_flock64)
#define XFS_IOC_UNRESVSP_32 _IOW('X', 41, struct compat_xfs_flock64)
#define XFS_IOC_RESVSP64_32 _IOW('X', 42, struct compat_xfs_flock64)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions
2022-01-11 23:22 [PATCHSET v2 0/3] xfs: kill dead ioctls for 5.17 Darrick J. Wong
2022-01-11 23:22 ` [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls Darrick J. Wong
2022-01-11 23:22 ` [PATCH 2/3] xfs: remove the XFS_IOC_{ALLOC,FREE}SP* definitions Darrick J. Wong
@ 2022-01-11 23:22 ` Darrick J. Wong
2022-01-13 3:53 ` Dave Chinner
2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2022-01-11 23:22 UTC (permalink / raw)
To: djwong; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
Remove the definitions for these ioctls, since the functionality (and,
weirdly, the 32-bit compat ioctl definitions) were removed from the
kernel in November 2019.
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_fs.h | 29 ++++-------------------------
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index 49c0e583d6bb..505533c43a92 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -92,21 +92,6 @@ struct getbmapx {
#define XFS_FMR_OWN_COW FMR_OWNER('X', 7) /* cow staging */
#define XFS_FMR_OWN_DEFECTIVE FMR_OWNER('X', 8) /* bad blocks */
-/*
- * Structure for XFS_IOC_FSSETDM.
- * For use by backup and restore programs to set the XFS on-disk inode
- * fields di_dmevmask and di_dmstate. These must be set to exactly and
- * only values previously obtained via xfs_bulkstat! (Specifically the
- * struct xfs_bstat fields bs_dmevmask and bs_dmstate.)
- */
-#ifndef HAVE_FSDMIDATA
-struct fsdmidata {
- __u32 fsd_dmevmask; /* corresponds to di_dmevmask */
- __u16 fsd_padding;
- __u16 fsd_dmstate; /* corresponds to di_dmstate */
-};
-#endif
-
/*
* File segment locking set data type for 64 bit access.
* Also used for all the RESV/FREE interfaces.
@@ -562,16 +547,10 @@ typedef struct xfs_fsop_handlereq {
/*
* Compound structures for passing args through Handle Request interfaces
- * xfs_fssetdm_by_handle, xfs_attrlist_by_handle, xfs_attrmulti_by_handle
- * - ioctls: XFS_IOC_FSSETDM_BY_HANDLE, XFS_IOC_ATTRLIST_BY_HANDLE, and
- * XFS_IOC_ATTRMULTI_BY_HANDLE
+ * xfs_attrlist_by_handle, xfs_attrmulti_by_handle
+ * - ioctls: XFS_IOC_ATTRLIST_BY_HANDLE, and XFS_IOC_ATTRMULTI_BY_HANDLE
*/
-typedef struct xfs_fsop_setdm_handlereq {
- struct xfs_fsop_handlereq hreq; /* handle information */
- struct fsdmidata __user *data; /* DMAPI data */
-} xfs_fsop_setdm_handlereq_t;
-
/*
* Flags passed in xfs_attr_multiop.am_flags for the attr ioctl interface.
*
@@ -789,7 +768,7 @@ struct xfs_scrub_metadata {
/* XFS_IOC_ALLOCSP64 ----- deprecated 36 */
/* XFS_IOC_FREESP64 ------ deprecated 37 */
#define XFS_IOC_GETBMAP _IOWR('X', 38, struct getbmap)
-#define XFS_IOC_FSSETDM _IOW ('X', 39, struct fsdmidata)
+/* XFS_IOC_FSSETDM ------- deprecated 39 */
#define XFS_IOC_RESVSP _IOW ('X', 40, struct xfs_flock64)
#define XFS_IOC_UNRESVSP _IOW ('X', 41, struct xfs_flock64)
#define XFS_IOC_RESVSP64 _IOW ('X', 42, struct xfs_flock64)
@@ -831,7 +810,7 @@ struct xfs_scrub_metadata {
#define XFS_IOC_FREEZE _IOWR('X', 119, int) /* aka FIFREEZE */
#define XFS_IOC_THAW _IOWR('X', 120, int) /* aka FITHAW */
-#define XFS_IOC_FSSETDM_BY_HANDLE _IOW ('X', 121, struct xfs_fsop_setdm_handlereq)
+/* XFS_IOC_FSSETDM_BY_HANDLE -- deprecated 121 */
#define XFS_IOC_ATTRLIST_BY_HANDLE _IOW ('X', 122, struct xfs_fsop_attrlist_handlereq)
#define XFS_IOC_ATTRMULTI_BY_HANDLE _IOW ('X', 123, struct xfs_fsop_attrmulti_handlereq)
#define XFS_IOC_FSGEOMETRY_V4 _IOR ('X', 124, struct xfs_fsop_geom_v4)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls
2022-01-11 23:22 ` [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls Darrick J. Wong
@ 2022-01-13 3:47 ` Dave Chinner
2022-01-13 13:15 ` Roger Willcocks
0 siblings, 1 reply; 7+ messages in thread
From: Dave Chinner @ 2022-01-13 3:47 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eric Sandeen, linux-xfs
On Tue, Jan 11, 2022 at 03:22:46PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> According to the glibc compat header for Irix 4, these ioctls originated
> in April 1991 as a (somewhat clunky) way to preallocate space at the end
> of a file on an EFS filesystem. XFS, which was released in Irix 5.3 in
> December 1993, picked up these ioctls to maintain compatibility and 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.
>
> See: CVE-2021-4155
>
> Augments: 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Looks good now.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions
2022-01-11 23:22 ` [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions Darrick J. Wong
@ 2022-01-13 3:53 ` Dave Chinner
0 siblings, 0 replies; 7+ messages in thread
From: Dave Chinner @ 2022-01-13 3:53 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On Tue, Jan 11, 2022 at 03:22:57PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Remove the definitions for these ioctls, since the functionality (and,
> weirdly, the 32-bit compat ioctl definitions) were removed from the
> kernel in November 2019.
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_fs.h | 29 ++++-------------------------
> 1 file changed, 4 insertions(+), 25 deletions(-)
Looks fine to me.
THe only user I know of is xfsdump, and it will only use this
functionality if a special CLI option is given to it. Given that
this would just be writing zeros as this is what will be in the
inodes that are backed up by xfsdump, I don't see it a big problem
if this fails now.
Nothing else out there is likely to be using this ioctl - the DMAPI
state was specific to a long dead proprietary SGI HSM product and
that's the only thing I know of that used this ioctl to set non-zero
values in the first place.
Hence I think removing this ioctl has very little risk of userspace
regression.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls
2022-01-13 3:47 ` Dave Chinner
@ 2022-01-13 13:15 ` Roger Willcocks
0 siblings, 0 replies; 7+ messages in thread
From: Roger Willcocks @ 2022-01-13 13:15 UTC (permalink / raw)
To: Dave Chinner; +Cc: Roger Willcocks, Darrick J. Wong, Eric Sandeen, linux-xfs
I will just mention in passing that we augment the XFS_IOC_ALLOC ioctl to perform explicit near-to-block allocation. I’m not suggesting you shouldn’t withdraw the ioctls, just referencing xkcd 1172.
—
Roger
> On 13 Jan 2022, at 03:47, Dave Chinner <david@fromorbit.com> wrote:
>
> On Tue, Jan 11, 2022 at 03:22:46PM -0800, Darrick J. Wong wrote:
>> From: Darrick J. Wong <djwong@kernel.org>
>>
>> According to the glibc compat header for Irix 4, these ioctls originated
>> in April 1991 as a (somewhat clunky) way to preallocate space at the end
>> of a file on an EFS filesystem. XFS, which was released in Irix 5.3 in
>> December 1993, picked up these ioctls to maintain compatibility and 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.
>>
>> See: CVE-2021-4155
>>
>> Augments: 983d8e60f508 ("xfs: map unwritten blocks in XFS_IOC_{ALLOC,FREE}SP just like fallocate")
>> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
>> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Looks good now.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> --
> Dave Chinner
> david@fromorbit.com
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-13 13:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 23:22 [PATCHSET v2 0/3] xfs: kill dead ioctls for 5.17 Darrick J. Wong
2022-01-11 23:22 ` [PATCH 1/3] xfs: kill the XFS_IOC_{ALLOC,FREE}SP* ioctls Darrick J. Wong
2022-01-13 3:47 ` Dave Chinner
2022-01-13 13:15 ` Roger Willcocks
2022-01-11 23:22 ` [PATCH 2/3] xfs: remove the XFS_IOC_{ALLOC,FREE}SP* definitions Darrick J. Wong
2022-01-11 23:22 ` [PATCH 3/3] xfs: remove the XFS_IOC_FSSETDM definitions Darrick J. Wong
2022-01-13 3:53 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.