xfs ioctl handler cleanups

xfs ioctl handler cleanups

[Christoph Hellwig]

Hi all,

I've been annoyed at the KASAN stack usage warnings in xfs_file_ioctl
again recently, so let's sort this out properly by splitting each
ioctl handler into a separate, well-named helper.  We already do that
for most, but this series completes it.

[PATCH 01/16] xfs: remove an outdated comment above xfs_file_ioctl

[Christoph Hellwig]

The return code sign flipping at the method boundary is long gone in XFS,
so remove this comment documenting an exception from it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 1b53701bebea..ec5412a9eb1d 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1209,12 +1209,6 @@ xfs_ioctl_fs_counts(
 #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.
- * So we don't "sign flip" like most other routines.  This means
- * true errors need to be returned as a negative value.
- */
 long
 xfs_file_ioctl(
 	struct file		*filp,
-- 
2.53.0

[PATCH 02/16] xfs: rename xfs_ioc_swapext to xfs_swapext

[Christoph Hellwig]

The usual convention is that the ioc_ prefix is used for direct ioctl
handlers that take a user pointer.  The current xfs_ioc_swapext does not
fit that pattern, so rename it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c   | 8 ++++----
 fs/xfs/xfs_ioctl.h   | 4 +---
 fs/xfs/xfs_ioctl32.c | 2 +-
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index ec5412a9eb1d..9a10645f5222 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -962,10 +962,10 @@ xfs_ioc_getbmap(
 }
 
 int
-xfs_ioc_swapext(
-	xfs_swapext_t	*sxp)
+xfs_swapext(
+	struct xfs_swapext	*sxp)
 {
-	xfs_inode_t     *ip, *tip;
+	struct xfs_inode	*ip, *tip;
 
 	/* Pull information for the target fd */
 	CLASS(fd, f)((int)sxp->sx_fdtarget);
@@ -1341,7 +1341,7 @@ xfs_file_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_ioc_swapext(&sxp);
+		error = xfs_swapext(&sxp);
 		mnt_drop_write_file(filp);
 		return error;
 	}
diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index f5ed5cf9d3df..e57d8f5148bf 100644
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -10,9 +10,7 @@ struct xfs_bstat;
 struct xfs_ibulk;
 struct xfs_inogrp;
 
-int
-xfs_ioc_swapext(
-	xfs_swapext_t	*sxp);
+int xfs_swapext(struct xfs_swapext *sxp);
 
 extern int
 xfs_fileattr_get(
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index c66e192448a8..250df2bbf214 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -475,7 +475,7 @@ xfs_file_compat_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_ioc_swapext(&sxp);
+		error = xfs_swapext(&sxp);
 		mnt_drop_write_file(filp);
 		return error;
 	}
-- 
2.53.0

[PATCH 03/16] xfs: rename xfs_ioctl_fs_counts to xfs_ioc_fs_counts

[Christoph Hellwig]

Match the naming scheme of most other ioctl handlers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 9a10645f5222..2d68b868b4e2 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1183,7 +1183,7 @@ xfs_ioctl_getset_resblocks(
 }
 
 static int
-xfs_ioctl_fs_counts(
+xfs_ioc_fs_counts(
 	struct xfs_mount	*mp,
 	struct xfs_fsop_counts __user	*uarg)
 {
@@ -1347,7 +1347,7 @@ xfs_file_ioctl(
 	}
 
 	case XFS_IOC_FSCOUNTS:
-		return xfs_ioctl_fs_counts(mp, arg);
+		return xfs_ioc_fs_counts(mp, arg);
 
 	case XFS_IOC_SET_RESBLKS:
 	case XFS_IOC_GET_RESBLKS:
-- 
2.53.0

[PATCH 04/16] xfs: rename xfs_ioctl_getset_resblocks to xfs_ioc_getset_resblocks

[Christoph Hellwig]

Match the naming scheme of most other ioctl handlers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 2d68b868b4e2..9d8ef646e0f8 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1144,7 +1144,7 @@ xfs_fs_eofblocks_from_user(
 }
 
 static int
-xfs_ioctl_getset_resblocks(
+xfs_ioc_getset_resblocks(
 	struct file		*filp,
 	unsigned int		cmd,
 	void __user		*arg)
@@ -1351,7 +1351,7 @@ xfs_file_ioctl(
 
 	case XFS_IOC_SET_RESBLKS:
 	case XFS_IOC_GET_RESBLKS:
-		return xfs_ioctl_getset_resblocks(filp, cmd, arg);
+		return xfs_ioc_getset_resblocks(filp, cmd, arg);
 
 	case XFS_IOC_FSGROWFSDATA: {
 		struct xfs_growfs_data in;
-- 
2.53.0

[PATCH 05/16] xfs: split out the handler for XFS_IOC_DIOINFO

[Christoph Hellwig]

Split out a helper for XFS_IOC_DIOINFO to keep the stack variables out of
xfs_file_ioctl and to clean up the main ioctl handler flow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 47 +++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 9d8ef646e0f8..2924d8d0f03f 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1200,6 +1200,32 @@ xfs_ioc_fs_counts(
 	return 0;
 }
 
+static int
+xfs_ioc_dioinfo(
+	struct file		*file,
+	void __user		*arg)
+{
+	struct kstat		st;
+	struct dioattr		da;
+	int			error;
+
+	error = vfs_getattr(&file->f_path, &st, STATX_DIOALIGN, 0);
+	if (error)
+		return error;
+
+	/*
+	 * Some userspace directly feeds the return value to posix_memalign,
+	 * which fails for values that are smaller than the pointer size.
+	 * Round up the value to not break userspace.
+	 */
+	da.d_mem = roundup(st.dio_mem_align, sizeof(void *));
+	da.d_miniosz = st.dio_offset_align;
+	da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
+	if (copy_to_user(arg, &da, sizeof(da)))
+		return -EFAULT;
+	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.
@@ -1238,26 +1264,9 @@ xfs_file_ioctl(
 	"%s should use fallocate; XFS_IOC_{ALLOC,FREE}SP ioctl unsupported",
 				current->comm);
 		return -ENOTTY;
-	case XFS_IOC_DIOINFO: {
-		struct kstat		st;
-		struct dioattr		da;
-
-		error = vfs_getattr(&filp->f_path, &st, STATX_DIOALIGN, 0);
-		if (error)
-			return error;
 
-		/*
-		 * Some userspace directly feeds the return value to
-		 * posix_memalign, which fails for values that are smaller than
-		 * the pointer size.  Round up the value to not break userspace.
-		 */
-		da.d_mem = roundup(st.dio_mem_align, sizeof(void *));
-		da.d_miniosz = st.dio_offset_align;
-		da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);
-		if (copy_to_user(arg, &da, sizeof(da)))
-			return -EFAULT;
-		return 0;
-	}
+	case XFS_IOC_DIOINFO:
+		return xfs_ioc_dioinfo(filp, arg);
 
 	case XFS_IOC_FSBULKSTAT_SINGLE:
 	case XFS_IOC_FSBULKSTAT:
-- 
2.53.0

[PATCH 06/16] xfs: split out the handlers for XFS_IOC_.*HANDLE

[Christoph Hellwig]

Split out helpers for XFS_IOC_.*HANDLE to keep the stack variables out of
xfs_file_ioctl and to clean up the main ioctl handler flow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 65 ++++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 2924d8d0f03f..6a7c570ea535 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1226,6 +1226,42 @@ xfs_ioc_dioinfo(
 	return 0;
 }
 
+static int
+xfs_ioc_find_handle(
+	unsigned int		cmd,
+	void __user		*arg)
+{
+	struct xfs_fsop_handlereq hreq;
+
+	if (copy_from_user(&hreq, arg, sizeof(hreq)))
+		return -EFAULT;
+	return xfs_find_handle(cmd, &hreq);
+}
+
+static int
+xfs_ioc_open_by_handle(
+	struct file		*file,
+	void __user		*arg)
+{
+	struct xfs_fsop_handlereq hreq;
+
+	if (copy_from_user(&hreq, arg, sizeof(hreq)))
+		return -EFAULT;
+	return xfs_open_by_handle(file, &hreq);
+}
+
+static int
+xfs_ioc_readlink_by_handle(
+	struct file		*file,
+	void __user		*arg)
+{
+	struct xfs_fsop_handlereq hreq;
+
+	if (copy_from_user(&hreq, arg, sizeof(hreq)))
+		return -EFAULT;
+	return xfs_readlink_by_handle(file, &hreq);
+}
+
 /*
  * 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.
@@ -1314,31 +1350,14 @@ xfs_file_ioctl(
 
 	case XFS_IOC_FD_TO_HANDLE:
 	case XFS_IOC_PATH_TO_HANDLE:
-	case XFS_IOC_PATH_TO_FSHANDLE: {
-		xfs_fsop_handlereq_t	hreq;
-
-		if (copy_from_user(&hreq, arg, sizeof(hreq)))
-			return -EFAULT;
-		return xfs_find_handle(cmd, &hreq);
-	}
-	case XFS_IOC_OPEN_BY_HANDLE: {
-		xfs_fsop_handlereq_t	hreq;
-
-		if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
-			return -EFAULT;
-		return xfs_open_by_handle(filp, &hreq);
-	}
-
-	case XFS_IOC_READLINK_BY_HANDLE: {
-		xfs_fsop_handlereq_t	hreq;
-
-		if (copy_from_user(&hreq, arg, sizeof(xfs_fsop_handlereq_t)))
-			return -EFAULT;
-		return xfs_readlink_by_handle(filp, &hreq);
-	}
+	case XFS_IOC_PATH_TO_FSHANDLE:
+		return xfs_ioc_find_handle(cmd, arg);
+	case XFS_IOC_OPEN_BY_HANDLE:
+		return xfs_ioc_open_by_handle(filp, arg);
+	case XFS_IOC_READLINK_BY_HANDLE:
+		return xfs_ioc_readlink_by_handle(filp, arg);
 	case XFS_IOC_ATTRLIST_BY_HANDLE:
 		return xfs_attrlist_by_handle(filp, arg);
-
 	case XFS_IOC_ATTRMULTI_BY_HANDLE:
 		return xfs_attrmulti_by_handle(filp, arg);
 
-- 
2.53.0

[PATCH 07/16] xfs: split out the handler for XFS_IOC_SWAPEXT

[Christoph Hellwig]

Split out a helper for XFS_IOC_SWAPEXT to keep the stack variables out of
xfs_file_ioctl and to clean up the main ioctl handler flow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 6a7c570ea535..eeff0678d5bf 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1262,6 +1262,25 @@ xfs_ioc_readlink_by_handle(
 	return xfs_readlink_by_handle(file, &hreq);
 }
 
+static int
+xfs_ioc_swapext(
+	struct file		*file,
+	void __user		*arg)
+{
+	struct xfs_swapext	sxp;
+	int			error;
+
+	if (copy_from_user(&sxp, arg, sizeof(sxp)))
+		return -EFAULT;
+
+	error = mnt_want_write_file(file);
+	if (error)
+		return error;
+	error = xfs_swapext(&sxp);
+	mnt_drop_write_file(file);
+	return error;
+}
+
 /*
  * 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.
@@ -1361,18 +1380,8 @@ xfs_file_ioctl(
 	case XFS_IOC_ATTRMULTI_BY_HANDLE:
 		return xfs_attrmulti_by_handle(filp, arg);
 
-	case XFS_IOC_SWAPEXT: {
-		struct xfs_swapext	sxp;
-
-		if (copy_from_user(&sxp, arg, sizeof(xfs_swapext_t)))
-			return -EFAULT;
-		error = mnt_want_write_file(filp);
-		if (error)
-			return error;
-		error = xfs_swapext(&sxp);
-		mnt_drop_write_file(filp);
-		return error;
-	}
+	case XFS_IOC_SWAPEXT:
+		return xfs_ioc_swapext(filp, arg);
 
 	case XFS_IOC_FSCOUNTS:
 		return xfs_ioc_fs_counts(mp, arg);
-- 
2.53.0

[PATCH 08/16] xfs: split out the handlers for XFS_IOC_FSGROWFS*

[Christoph Hellwig]

Split out helpers for XFS_IOC_FSGROWFS* to keep the stack variables out
of xfs_file_ioctl and to clean up the main ioctl handler flow.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl.c | 107 ++++++++++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 41 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index eeff0678d5bf..0b5ae79e4aaf 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1281,6 +1281,66 @@ xfs_ioc_swapext(
 	return error;
 }
 
+static int
+xfs_ioc_growfs_data(
+	struct file		*file,
+	struct xfs_mount	*mp,
+	void __user		*arg)
+{
+	struct xfs_growfs_data	in;
+	int			error;
+
+	if (copy_from_user(&in, arg, sizeof(in)))
+		return -EFAULT;
+
+	error = mnt_want_write_file(file);
+	if (error)
+		return error;
+	error = xfs_growfs_data(mp, &in);
+	mnt_drop_write_file(file);
+	return error;
+}
+
+static int
+xfs_ioc_growfs_log(
+	struct file		*file,
+	struct xfs_mount	*mp,
+	void  __user		*arg)
+{
+	struct xfs_growfs_log	in;
+	int			error;
+
+	if (copy_from_user(&in, arg, sizeof(in)))
+		return -EFAULT;
+
+	error = mnt_want_write_file(file);
+	if (error)
+		return error;
+	error = xfs_growfs_log(mp, &in);
+	mnt_drop_write_file(file);
+	return error;
+}
+
+static int
+xfs_ioc_growfs_rt(
+	struct file		*file,
+	struct xfs_mount	*mp,
+	void __user		*arg)
+{
+	struct xfs_growfs_rt	in;
+	int			error;
+
+	if (copy_from_user(&in, arg, sizeof(in)))
+		return -EFAULT;
+
+	error = mnt_want_write_file(file);
+	if (error)
+		return error;
+	error = xfs_growfs_rt(mp, &in);
+	mnt_drop_write_file(file);
+	return error;
+}
+
 /*
  * 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.
@@ -1390,47 +1450,12 @@ xfs_file_ioctl(
 	case XFS_IOC_GET_RESBLKS:
 		return xfs_ioc_getset_resblocks(filp, cmd, arg);
 
-	case XFS_IOC_FSGROWFSDATA: {
-		struct xfs_growfs_data in;
-
-		if (copy_from_user(&in, arg, sizeof(in)))
-			return -EFAULT;
-
-		error = mnt_want_write_file(filp);
-		if (error)
-			return error;
-		error = xfs_growfs_data(mp, &in);
-		mnt_drop_write_file(filp);
-		return error;
-	}
-
-	case XFS_IOC_FSGROWFSLOG: {
-		struct xfs_growfs_log in;
-
-		if (copy_from_user(&in, arg, sizeof(in)))
-			return -EFAULT;
-
-		error = mnt_want_write_file(filp);
-		if (error)
-			return error;
-		error = xfs_growfs_log(mp, &in);
-		mnt_drop_write_file(filp);
-		return error;
-	}
-
-	case XFS_IOC_FSGROWFSRT: {
-		xfs_growfs_rt_t in;
-
-		if (copy_from_user(&in, arg, sizeof(in)))
-			return -EFAULT;
-
-		error = mnt_want_write_file(filp);
-		if (error)
-			return error;
-		error = xfs_growfs_rt(mp, &in);
-		mnt_drop_write_file(filp);
-		return error;
-	}
+	case XFS_IOC_FSGROWFSDATA:
+		return xfs_ioc_growfs_data(filp, mp, arg);
+	case XFS_IOC_FSGROWFSLOG:
+		return xfs_ioc_growfs_log(filp, mp, arg);
+	case XFS_IOC_FSGROWFSRT:
+		return xfs_ioc_growfs_rt(filp, mp, arg);
 
 	case XFS_IOC_GOINGDOWN: {
 		uint32_t in;
-- 
2.53.0

xfs ioctl handler cleanups

[Christoph Hellwig]

Hi all,

I've been annoyed at the KASAN stack usage warnings in xfs_file_ioctl
again recently, so let's sort this out properly by splitting each
ioctl handler into a separate, well-named helper.  We already do that
for most, but this series completes it.

Re: xfs ioctl handler cleanups

[Christoph Hellwig]

Sorry, sending this got interrupted.  The full version has been sent
out now separately.

Re: xfs ioctl handler cleanups

[Darrick J. Wong]

On Wed, Jul 29, 2026 at 03:02:59PM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> I've been annoyed at the KASAN stack usage warnings in xfs_file_ioctl
> again recently, so let's sort this out properly by splitting each
> ioctl handler into a separate, well-named helper.  We already do that
> for most, but this series completes it.

The whole series looks fine to me:
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D