[PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO

[PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO

[Daan De Meyer]

On 64-bit kernels with 32-bit userspace, struct btrfs_ioctl_timespec is
laid out as 16 bytes (8B sec + 4B nsec + 4B trailing padding) instead of
the 12 bytes a 32-bit userspace expects, because the surrounding struct
is not packed. As a result, struct btrfs_ioctl_get_subvol_info_args has
a different size and layout in 32-bit userspace than in the 64-bit
kernel, and BTRFS_IOC_GET_SUBVOL_INFO returns garbage to 32-bit callers.

Mirror what was done for BTRFS_IOC_SET_RECEIVED_SUBVOL: add a packed
btrfs_ioctl_get_subvol_info_args_32 with btrfs_ioctl_timespec_32 fields,
define BTRFS_IOC_GET_SUBVOL_INFO_32 with that struct as the size
argument, factor the existing handler into a shared _btrfs_ioctl_get_
subvol_info() helper, and add btrfs_ioctl_get_subvol_info_32() which
fills the kernel struct and translates field-by-field into the 32-bit
struct before copy_to_user().

Signed-off-by: Daan De Meyer <daan@amutable.com>
---
 fs/btrfs/ioctl.c | 112 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 99 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index a39460bf68a7..31be2590f1b2 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -82,6 +82,30 @@ struct btrfs_ioctl_received_subvol_args_32 {
 
 #define BTRFS_IOC_SET_RECEIVED_SUBVOL_32 _IOWR(BTRFS_IOCTL_MAGIC, 37, \
 				struct btrfs_ioctl_received_subvol_args_32)
+
+struct btrfs_ioctl_get_subvol_info_args_32 {
+	__u64 treeid;
+	char name[BTRFS_VOL_NAME_MAX + 1];
+	__u64 parent_id;
+	__u64 dirid;
+	__u64 generation;
+	__u64 flags;
+	__u8 uuid[BTRFS_UUID_SIZE];
+	__u8 parent_uuid[BTRFS_UUID_SIZE];
+	__u8 received_uuid[BTRFS_UUID_SIZE];
+	__u64 ctransid;
+	__u64 otransid;
+	__u64 stransid;
+	__u64 rtransid;
+	struct btrfs_ioctl_timespec_32 ctime;
+	struct btrfs_ioctl_timespec_32 otime;
+	struct btrfs_ioctl_timespec_32 stime;
+	struct btrfs_ioctl_timespec_32 rtime;
+	__u64 reserved[8];
+} __attribute__ ((__packed__));
+
+#define BTRFS_IOC_GET_SUBVOL_INFO_32 _IOR(BTRFS_IOCTL_MAGIC, 60, \
+				struct btrfs_ioctl_get_subvol_info_args_32)
 #endif
 
 #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
@@ -1945,9 +1969,9 @@ static int btrfs_ioctl_ino_lookup_user(struct file *file, void __user *argp)
 }
 
 /* Get the subvolume information in BTRFS_ROOT_ITEM and BTRFS_ROOT_BACKREF */
-static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
+static int _btrfs_ioctl_get_subvol_info(struct inode *inode,
+					struct btrfs_ioctl_get_subvol_info_args *subvol_info)
 {
-	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
 	struct btrfs_fs_info *fs_info;
 	struct btrfs_root *root;
 	struct btrfs_path *path;
@@ -1964,12 +1988,6 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 	if (!path)
 		return -ENOMEM;
 
-	subvol_info = kzalloc_obj(*subvol_info);
-	if (!subvol_info) {
-		btrfs_free_path(path);
-		return -ENOMEM;
-	}
-
 	fs_info = BTRFS_I(inode)->root->fs_info;
 
 	/* Get root_item of inode's subvolume */
@@ -2048,15 +2066,79 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
 		}
 	}
 
-	btrfs_free_path(path);
-	path = NULL;
-	if (copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
-		ret = -EFAULT;
-
 out:
 	btrfs_put_root(root);
 out_free:
 	btrfs_free_path(path);
+	return ret;
+}
+
+#ifdef CONFIG_64BIT
+static int btrfs_ioctl_get_subvol_info_32(struct inode *inode, void __user *argp)
+{
+	struct btrfs_ioctl_get_subvol_info_args *subvol_info = NULL;
+	struct btrfs_ioctl_get_subvol_info_args_32 *subvol_info_32 = NULL;
+	int ret;
+
+	subvol_info = kzalloc_obj(*subvol_info);
+	if (!subvol_info)
+		return -ENOMEM;
+
+	subvol_info_32 = kzalloc_obj(*subvol_info_32);
+	if (!subvol_info_32) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
+	if (ret)
+		goto out;
+
+	subvol_info_32->treeid = subvol_info->treeid;
+	memcpy(subvol_info_32->name, subvol_info->name, sizeof(subvol_info_32->name));
+	subvol_info_32->parent_id = subvol_info->parent_id;
+	subvol_info_32->dirid = subvol_info->dirid;
+	subvol_info_32->generation = subvol_info->generation;
+	subvol_info_32->flags = subvol_info->flags;
+	memcpy(subvol_info_32->uuid, subvol_info->uuid, BTRFS_UUID_SIZE);
+	memcpy(subvol_info_32->parent_uuid, subvol_info->parent_uuid, BTRFS_UUID_SIZE);
+	memcpy(subvol_info_32->received_uuid, subvol_info->received_uuid, BTRFS_UUID_SIZE);
+	subvol_info_32->ctransid = subvol_info->ctransid;
+	subvol_info_32->otransid = subvol_info->otransid;
+	subvol_info_32->stransid = subvol_info->stransid;
+	subvol_info_32->rtransid = subvol_info->rtransid;
+	subvol_info_32->ctime.sec = subvol_info->ctime.sec;
+	subvol_info_32->ctime.nsec = subvol_info->ctime.nsec;
+	subvol_info_32->otime.sec = subvol_info->otime.sec;
+	subvol_info_32->otime.nsec = subvol_info->otime.nsec;
+	subvol_info_32->stime.sec = subvol_info->stime.sec;
+	subvol_info_32->stime.nsec = subvol_info->stime.nsec;
+	subvol_info_32->rtime.sec = subvol_info->rtime.sec;
+	subvol_info_32->rtime.nsec = subvol_info->rtime.nsec;
+
+	if (copy_to_user(argp, subvol_info_32, sizeof(*subvol_info_32)))
+		ret = -EFAULT;
+
+out:
+	kfree(subvol_info_32);
+	kfree(subvol_info);
+	return ret;
+}
+#endif
+
+static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
+{
+	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
+	int ret;
+
+	subvol_info = kzalloc_obj(*subvol_info);
+	if (!subvol_info)
+		return -ENOMEM;
+
+	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
+	if (!ret && copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
+		ret = -EFAULT;
+
 	kfree(subvol_info);
 	return ret;
 }
@@ -5273,6 +5355,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_set_features(file, argp);
 	case BTRFS_IOC_GET_SUBVOL_INFO:
 		return btrfs_ioctl_get_subvol_info(inode, argp);
+#ifdef CONFIG_64BIT
+	case BTRFS_IOC_GET_SUBVOL_INFO_32:
+		return btrfs_ioctl_get_subvol_info_32(inode, argp);
+#endif
 	case BTRFS_IOC_GET_SUBVOL_ROOTREF:
 		return btrfs_ioctl_get_subvol_rootref(root, argp);
 	case BTRFS_IOC_INO_LOOKUP_USER:
-- 
2.54.0

Re: [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO

[Qu Wenruo]

在 2026/5/21 17:21, Daan De Meyer 写道:
> On 64-bit kernels with 32-bit userspace, struct btrfs_ioctl_timespec is
> laid out as 16 bytes (8B sec + 4B nsec + 4B trailing padding) instead of
> the 12 bytes a 32-bit userspace expects, because the surrounding struct
> is not packed. As a result, struct btrfs_ioctl_get_subvol_info_args has
> a different size and layout in 32-bit userspace than in the 64-bit
> kernel, and BTRFS_IOC_GET_SUBVOL_INFO returns garbage to 32-bit callers.
> 
> Mirror what was done for BTRFS_IOC_SET_RECEIVED_SUBVOL: add a packed
> btrfs_ioctl_get_subvol_info_args_32 with btrfs_ioctl_timespec_32 fields,
> define BTRFS_IOC_GET_SUBVOL_INFO_32 with that struct as the size
> argument, factor the existing handler into a shared _btrfs_ioctl_get_
> subvol_info() helper, and add btrfs_ioctl_get_subvol_info_32() which
> fills the kernel struct and translates field-by-field into the 32-bit
> struct before copy_to_user().
> 
> Signed-off-by: Daan De Meyer <daan@amutable.com>

Unfortunately this is causing libbtrfsutils (from btrfs-progs) to 
regress on the following tests:

======================================================================
FAIL: test_subvolume_get_info_unprivileged 
(test_subvolume.TestSubvolume.test_subvolume_get_info_unprivileged)
----------------------------------------------------------------------
Traceback (most recent call last):
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 336, in test_subvolume_get_info_unprivileged
     self._test_subvolume_info(subvol, snapshot)
     ~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 196, in _test_subvolume_info
     self.assertEqual(info.id, 256)
     ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
AssertionError: 140488005556880 != 256

======================================================================
FAIL: test_subvolume_info_unprivileged 
(test_subvolume.TestSubvolume.test_subvolume_info_unprivileged)
----------------------------------------------------------------------
Traceback (most recent call last):
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 318, in test_subvolume_info_unprivileged
     self._test_subvolume_info(subvol, snapshot)
     ~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 196, in _test_subvolume_info
     self.assertEqual(info.id, 256)
     ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
AssertionError: 140488005503648 != 256

======================================================================
FAIL: test_subvolume_iterator_unprivileged 
(test_subvolume.TestSubvolume.test_subvolume_iterator_unprivileged)
----------------------------------------------------------------------
Traceback (most recent call last):
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 835, in test_subvolume_iterator_unprivileged
     self._test_subvolume_iterator()
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
   File 
"/home/adam/btrfs-progs/libbtrfsutil/python/tests/test_subvolume.py", 
line 700, in _test_subvolume_iterator
     self.assertEqual(subvol.id, 256)
     ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
AssertionError: 0 != 256

----------------------------------------------------------------------
Ran 38 tests in 126.919s

And I have confirmed that, at that commit the libbtrfsutils selftest 
fails, and at the commit ("btrfs: derive f_fsid from on-disk fsid and 
dev_t") just before this one, the self test passes as usual.

I'm afraid this one needs to be removed from for-next branch, as it is 
causing regression for 64bit systems.

Thanks,
Qu

> ---
>   fs/btrfs/ioctl.c | 112 +++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 99 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a39460bf68a7..31be2590f1b2 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -82,6 +82,30 @@ struct btrfs_ioctl_received_subvol_args_32 {
>   
>   #define BTRFS_IOC_SET_RECEIVED_SUBVOL_32 _IOWR(BTRFS_IOCTL_MAGIC, 37, \
>   				struct btrfs_ioctl_received_subvol_args_32)
> +
> +struct btrfs_ioctl_get_subvol_info_args_32 {
> +	__u64 treeid;
> +	char name[BTRFS_VOL_NAME_MAX + 1];
> +	__u64 parent_id;
> +	__u64 dirid;
> +	__u64 generation;
> +	__u64 flags;
> +	__u8 uuid[BTRFS_UUID_SIZE];
> +	__u8 parent_uuid[BTRFS_UUID_SIZE];
> +	__u8 received_uuid[BTRFS_UUID_SIZE];
> +	__u64 ctransid;
> +	__u64 otransid;
> +	__u64 stransid;
> +	__u64 rtransid;
> +	struct btrfs_ioctl_timespec_32 ctime;
> +	struct btrfs_ioctl_timespec_32 otime;
> +	struct btrfs_ioctl_timespec_32 stime;
> +	struct btrfs_ioctl_timespec_32 rtime;
> +	__u64 reserved[8];
> +} __attribute__ ((__packed__));
> +
> +#define BTRFS_IOC_GET_SUBVOL_INFO_32 _IOR(BTRFS_IOCTL_MAGIC, 60, \
> +				struct btrfs_ioctl_get_subvol_info_args_32)
>   #endif
>   
>   #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
> @@ -1945,9 +1969,9 @@ static int btrfs_ioctl_ino_lookup_user(struct file *file, void __user *argp)
>   }
>   
>   /* Get the subvolume information in BTRFS_ROOT_ITEM and BTRFS_ROOT_BACKREF */
> -static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
> +static int _btrfs_ioctl_get_subvol_info(struct inode *inode,
> +					struct btrfs_ioctl_get_subvol_info_args *subvol_info)
>   {
> -	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
>   	struct btrfs_fs_info *fs_info;
>   	struct btrfs_root *root;
>   	struct btrfs_path *path;
> @@ -1964,12 +1988,6 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>   	if (!path)
>   		return -ENOMEM;
>   
> -	subvol_info = kzalloc_obj(*subvol_info);
> -	if (!subvol_info) {
> -		btrfs_free_path(path);
> -		return -ENOMEM;
> -	}
> -
>   	fs_info = BTRFS_I(inode)->root->fs_info;
>   
>   	/* Get root_item of inode's subvolume */
> @@ -2048,15 +2066,79 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>   		}
>   	}
>   
> -	btrfs_free_path(path);
> -	path = NULL;
> -	if (copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
> -		ret = -EFAULT;
> -
>   out:
>   	btrfs_put_root(root);
>   out_free:
>   	btrfs_free_path(path);
> +	return ret;
> +}
> +
> +#ifdef CONFIG_64BIT
> +static int btrfs_ioctl_get_subvol_info_32(struct inode *inode, void __user *argp)
> +{
> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info = NULL;
> +	struct btrfs_ioctl_get_subvol_info_args_32 *subvol_info_32 = NULL;
> +	int ret;
> +
> +	subvol_info = kzalloc_obj(*subvol_info);
> +	if (!subvol_info)
> +		return -ENOMEM;
> +
> +	subvol_info_32 = kzalloc_obj(*subvol_info_32);
> +	if (!subvol_info_32) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
> +	if (ret)
> +		goto out;
> +
> +	subvol_info_32->treeid = subvol_info->treeid;
> +	memcpy(subvol_info_32->name, subvol_info->name, sizeof(subvol_info_32->name));
> +	subvol_info_32->parent_id = subvol_info->parent_id;
> +	subvol_info_32->dirid = subvol_info->dirid;
> +	subvol_info_32->generation = subvol_info->generation;
> +	subvol_info_32->flags = subvol_info->flags;
> +	memcpy(subvol_info_32->uuid, subvol_info->uuid, BTRFS_UUID_SIZE);
> +	memcpy(subvol_info_32->parent_uuid, subvol_info->parent_uuid, BTRFS_UUID_SIZE);
> +	memcpy(subvol_info_32->received_uuid, subvol_info->received_uuid, BTRFS_UUID_SIZE);
> +	subvol_info_32->ctransid = subvol_info->ctransid;
> +	subvol_info_32->otransid = subvol_info->otransid;
> +	subvol_info_32->stransid = subvol_info->stransid;
> +	subvol_info_32->rtransid = subvol_info->rtransid;
> +	subvol_info_32->ctime.sec = subvol_info->ctime.sec;
> +	subvol_info_32->ctime.nsec = subvol_info->ctime.nsec;
> +	subvol_info_32->otime.sec = subvol_info->otime.sec;
> +	subvol_info_32->otime.nsec = subvol_info->otime.nsec;
> +	subvol_info_32->stime.sec = subvol_info->stime.sec;
> +	subvol_info_32->stime.nsec = subvol_info->stime.nsec;
> +	subvol_info_32->rtime.sec = subvol_info->rtime.sec;
> +	subvol_info_32->rtime.nsec = subvol_info->rtime.nsec;
> +
> +	if (copy_to_user(argp, subvol_info_32, sizeof(*subvol_info_32)))
> +		ret = -EFAULT;
> +
> +out:
> +	kfree(subvol_info_32);
> +	kfree(subvol_info);
> +	return ret;
> +}
> +#endif
> +
> +static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
> +{
> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
> +	int ret;
> +
> +	subvol_info = kzalloc_obj(*subvol_info);
> +	if (!subvol_info)
> +		return -ENOMEM;
> +
> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
> +	if (!ret && copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
> +		ret = -EFAULT;
> +
>   	kfree(subvol_info);
>   	return ret;
>   }
> @@ -5273,6 +5355,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>   		return btrfs_ioctl_set_features(file, argp);
>   	case BTRFS_IOC_GET_SUBVOL_INFO:
>   		return btrfs_ioctl_get_subvol_info(inode, argp);
> +#ifdef CONFIG_64BIT
> +	case BTRFS_IOC_GET_SUBVOL_INFO_32:
> +		return btrfs_ioctl_get_subvol_info_32(inode, argp);
> +#endif
>   	case BTRFS_IOC_GET_SUBVOL_ROOTREF:
>   		return btrfs_ioctl_get_subvol_rootref(root, argp);
>   	case BTRFS_IOC_INO_LOOKUP_USER:

Re: [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO

[Miquel Sabaté Solà]

[-- Attachment #1: Type: text/plain, Size: 6685 bytes --]

Hi,

Just a small comment from my side, but feel free to ignore it :)

Daan De Meyer @ 2026-05-21 07:51 GMT:

> On 64-bit kernels with 32-bit userspace, struct btrfs_ioctl_timespec is
> laid out as 16 bytes (8B sec + 4B nsec + 4B trailing padding) instead of
> the 12 bytes a 32-bit userspace expects, because the surrounding struct
> is not packed. As a result, struct btrfs_ioctl_get_subvol_info_args has
> a different size and layout in 32-bit userspace than in the 64-bit
> kernel, and BTRFS_IOC_GET_SUBVOL_INFO returns garbage to 32-bit callers.
>
> Mirror what was done for BTRFS_IOC_SET_RECEIVED_SUBVOL: add a packed
> btrfs_ioctl_get_subvol_info_args_32 with btrfs_ioctl_timespec_32 fields,
> define BTRFS_IOC_GET_SUBVOL_INFO_32 with that struct as the size
> argument, factor the existing handler into a shared _btrfs_ioctl_get_
> subvol_info() helper, and add btrfs_ioctl_get_subvol_info_32() which
> fills the kernel struct and translates field-by-field into the 32-bit
> struct before copy_to_user().
>
> Signed-off-by: Daan De Meyer <daan@amutable.com>
> ---
>  fs/btrfs/ioctl.c | 112 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 99 insertions(+), 13 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index a39460bf68a7..31be2590f1b2 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -82,6 +82,30 @@ struct btrfs_ioctl_received_subvol_args_32 {
>
>  #define BTRFS_IOC_SET_RECEIVED_SUBVOL_32 _IOWR(BTRFS_IOCTL_MAGIC, 37, \
>  				struct btrfs_ioctl_received_subvol_args_32)
> +
> +struct btrfs_ioctl_get_subvol_info_args_32 {
> +	__u64 treeid;
> +	char name[BTRFS_VOL_NAME_MAX + 1];
> +	__u64 parent_id;
> +	__u64 dirid;
> +	__u64 generation;
> +	__u64 flags;
> +	__u8 uuid[BTRFS_UUID_SIZE];
> +	__u8 parent_uuid[BTRFS_UUID_SIZE];
> +	__u8 received_uuid[BTRFS_UUID_SIZE];
> +	__u64 ctransid;
> +	__u64 otransid;
> +	__u64 stransid;
> +	__u64 rtransid;
> +	struct btrfs_ioctl_timespec_32 ctime;
> +	struct btrfs_ioctl_timespec_32 otime;
> +	struct btrfs_ioctl_timespec_32 stime;
> +	struct btrfs_ioctl_timespec_32 rtime;
> +	__u64 reserved[8];
> +} __attribute__ ((__packed__));
> +
> +#define BTRFS_IOC_GET_SUBVOL_INFO_32 _IOR(BTRFS_IOCTL_MAGIC, 60, \
> +				struct btrfs_ioctl_get_subvol_info_args_32)
>  #endif
>
>  #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
> @@ -1945,9 +1969,9 @@ static int btrfs_ioctl_ino_lookup_user(struct file *file, void __user *argp)
>  }
>
>  /* Get the subvolume information in BTRFS_ROOT_ITEM and BTRFS_ROOT_BACKREF */
> -static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
> +static int _btrfs_ioctl_get_subvol_info(struct inode *inode,
> +					struct btrfs_ioctl_get_subvol_info_args *subvol_info)
>  {
> -	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
>  	struct btrfs_fs_info *fs_info;
>  	struct btrfs_root *root;
>  	struct btrfs_path *path;
> @@ -1964,12 +1988,6 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>  	if (!path)
>  		return -ENOMEM;
>
> -	subvol_info = kzalloc_obj(*subvol_info);
> -	if (!subvol_info) {
> -		btrfs_free_path(path);
> -		return -ENOMEM;
> -	}
> -
>  	fs_info = BTRFS_I(inode)->root->fs_info;
>
>  	/* Get root_item of inode's subvolume */
> @@ -2048,15 +2066,79 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>  		}
>  	}
>
> -	btrfs_free_path(path);
> -	path = NULL;
> -	if (copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
> -		ret = -EFAULT;
> -
>  out:
>  	btrfs_put_root(root);
>  out_free:
>  	btrfs_free_path(path);
> +	return ret;
> +}
> +
> +#ifdef CONFIG_64BIT
> +static int btrfs_ioctl_get_subvol_info_32(struct inode *inode, void __user *argp)
> +{
> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info = NULL;
> +	struct btrfs_ioctl_get_subvol_info_args_32 *subvol_info_32 = NULL;

Maybe these two variables could be declared with the AUTO_KFREE helper,
so you could remove goto's and the cleanup section in the end.

> +	int ret;
> +
> +	subvol_info = kzalloc_obj(*subvol_info);
> +	if (!subvol_info)
> +		return -ENOMEM;
> +
> +	subvol_info_32 = kzalloc_obj(*subvol_info_32);
> +	if (!subvol_info_32) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
> +	if (ret)
> +		goto out;
> +
> +	subvol_info_32->treeid = subvol_info->treeid;
> +	memcpy(subvol_info_32->name, subvol_info->name, sizeof(subvol_info_32->name));
> +	subvol_info_32->parent_id = subvol_info->parent_id;
> +	subvol_info_32->dirid = subvol_info->dirid;
> +	subvol_info_32->generation = subvol_info->generation;
> +	subvol_info_32->flags = subvol_info->flags;
> +	memcpy(subvol_info_32->uuid, subvol_info->uuid, BTRFS_UUID_SIZE);
> +	memcpy(subvol_info_32->parent_uuid, subvol_info->parent_uuid, BTRFS_UUID_SIZE);
> +	memcpy(subvol_info_32->received_uuid, subvol_info->received_uuid, BTRFS_UUID_SIZE);
> +	subvol_info_32->ctransid = subvol_info->ctransid;
> +	subvol_info_32->otransid = subvol_info->otransid;
> +	subvol_info_32->stransid = subvol_info->stransid;
> +	subvol_info_32->rtransid = subvol_info->rtransid;
> +	subvol_info_32->ctime.sec = subvol_info->ctime.sec;
> +	subvol_info_32->ctime.nsec = subvol_info->ctime.nsec;
> +	subvol_info_32->otime.sec = subvol_info->otime.sec;
> +	subvol_info_32->otime.nsec = subvol_info->otime.nsec;
> +	subvol_info_32->stime.sec = subvol_info->stime.sec;
> +	subvol_info_32->stime.nsec = subvol_info->stime.nsec;
> +	subvol_info_32->rtime.sec = subvol_info->rtime.sec;
> +	subvol_info_32->rtime.nsec = subvol_info->rtime.nsec;
> +
> +	if (copy_to_user(argp, subvol_info_32, sizeof(*subvol_info_32)))
> +		ret = -EFAULT;
> +
> +out:
> +	kfree(subvol_info_32);
> +	kfree(subvol_info);
> +	return ret;
> +}
> +#endif
> +
> +static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
> +{
> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
> +	int ret;
> +
> +	subvol_info = kzalloc_obj(*subvol_info);
> +	if (!subvol_info)
> +		return -ENOMEM;
> +
> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
> +	if (!ret && copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
> +		ret = -EFAULT;
> +
>  	kfree(subvol_info);
>  	return ret;
>  }
> @@ -5273,6 +5355,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>  		return btrfs_ioctl_set_features(file, argp);
>  	case BTRFS_IOC_GET_SUBVOL_INFO:
>  		return btrfs_ioctl_get_subvol_info(inode, argp);
> +#ifdef CONFIG_64BIT
> +	case BTRFS_IOC_GET_SUBVOL_INFO_32:
> +		return btrfs_ioctl_get_subvol_info_32(inode, argp);
> +#endif
>  	case BTRFS_IOC_GET_SUBVOL_ROOTREF:
>  		return btrfs_ioctl_get_subvol_rootref(root, argp);
>  	case BTRFS_IOC_INO_LOOKUP_USER:

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]

Re: [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO

[Miquel Sabaté Solà]

[-- Attachment #1: Type: text/plain, Size: 7256 bytes --]

Hi,

Just realized I'm commenting on an old thread and that the AUTO_KFREE
suggestion was already applied by David.

Sorry for the noise.

Miquel Sabaté Solà @ 2026-06-17 13:18 +02:

> Hi,
>
> Just a small comment from my side, but feel free to ignore it :)
>
> Daan De Meyer @ 2026-05-21 07:51 GMT:
>
>> On 64-bit kernels with 32-bit userspace, struct btrfs_ioctl_timespec is
>> laid out as 16 bytes (8B sec + 4B nsec + 4B trailing padding) instead of
>> the 12 bytes a 32-bit userspace expects, because the surrounding struct
>> is not packed. As a result, struct btrfs_ioctl_get_subvol_info_args has
>> a different size and layout in 32-bit userspace than in the 64-bit
>> kernel, and BTRFS_IOC_GET_SUBVOL_INFO returns garbage to 32-bit callers.
>>
>> Mirror what was done for BTRFS_IOC_SET_RECEIVED_SUBVOL: add a packed
>> btrfs_ioctl_get_subvol_info_args_32 with btrfs_ioctl_timespec_32 fields,
>> define BTRFS_IOC_GET_SUBVOL_INFO_32 with that struct as the size
>> argument, factor the existing handler into a shared _btrfs_ioctl_get_
>> subvol_info() helper, and add btrfs_ioctl_get_subvol_info_32() which
>> fills the kernel struct and translates field-by-field into the 32-bit
>> struct before copy_to_user().
>>
>> Signed-off-by: Daan De Meyer <daan@amutable.com>
>> ---
>>  fs/btrfs/ioctl.c | 112 +++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 99 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index a39460bf68a7..31be2590f1b2 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -82,6 +82,30 @@ struct btrfs_ioctl_received_subvol_args_32 {
>>
>>  #define BTRFS_IOC_SET_RECEIVED_SUBVOL_32 _IOWR(BTRFS_IOCTL_MAGIC, 37, \
>>  				struct btrfs_ioctl_received_subvol_args_32)
>> +
>> +struct btrfs_ioctl_get_subvol_info_args_32 {
>> +	__u64 treeid;
>> +	char name[BTRFS_VOL_NAME_MAX + 1];
>> +	__u64 parent_id;
>> +	__u64 dirid;
>> +	__u64 generation;
>> +	__u64 flags;
>> +	__u8 uuid[BTRFS_UUID_SIZE];
>> +	__u8 parent_uuid[BTRFS_UUID_SIZE];
>> +	__u8 received_uuid[BTRFS_UUID_SIZE];
>> +	__u64 ctransid;
>> +	__u64 otransid;
>> +	__u64 stransid;
>> +	__u64 rtransid;
>> +	struct btrfs_ioctl_timespec_32 ctime;
>> +	struct btrfs_ioctl_timespec_32 otime;
>> +	struct btrfs_ioctl_timespec_32 stime;
>> +	struct btrfs_ioctl_timespec_32 rtime;
>> +	__u64 reserved[8];
>> +} __attribute__ ((__packed__));
>> +
>> +#define BTRFS_IOC_GET_SUBVOL_INFO_32 _IOR(BTRFS_IOCTL_MAGIC, 60, \
>> +				struct btrfs_ioctl_get_subvol_info_args_32)
>>  #endif
>>
>>  #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT)
>> @@ -1945,9 +1969,9 @@ static int btrfs_ioctl_ino_lookup_user(struct file *file, void __user *argp)
>>  }
>>
>>  /* Get the subvolume information in BTRFS_ROOT_ITEM and BTRFS_ROOT_BACKREF */
>> -static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>> +static int _btrfs_ioctl_get_subvol_info(struct inode *inode,
>> +					struct btrfs_ioctl_get_subvol_info_args *subvol_info)
>>  {
>> -	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
>>  	struct btrfs_fs_info *fs_info;
>>  	struct btrfs_root *root;
>>  	struct btrfs_path *path;
>> @@ -1964,12 +1988,6 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>>  	if (!path)
>>  		return -ENOMEM;
>>
>> -	subvol_info = kzalloc_obj(*subvol_info);
>> -	if (!subvol_info) {
>> -		btrfs_free_path(path);
>> -		return -ENOMEM;
>> -	}
>> -
>>  	fs_info = BTRFS_I(inode)->root->fs_info;
>>
>>  	/* Get root_item of inode's subvolume */
>> @@ -2048,15 +2066,79 @@ static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>>  		}
>>  	}
>>
>> -	btrfs_free_path(path);
>> -	path = NULL;
>> -	if (copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
>> -		ret = -EFAULT;
>> -
>>  out:
>>  	btrfs_put_root(root);
>>  out_free:
>>  	btrfs_free_path(path);
>> +	return ret;
>> +}
>> +
>> +#ifdef CONFIG_64BIT
>> +static int btrfs_ioctl_get_subvol_info_32(struct inode *inode, void __user *argp)
>> +{
>> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info = NULL;
>> +	struct btrfs_ioctl_get_subvol_info_args_32 *subvol_info_32 = NULL;
>
> Maybe these two variables could be declared with the AUTO_KFREE helper,
> so you could remove goto's and the cleanup section in the end.
>
>> +	int ret;
>> +
>> +	subvol_info = kzalloc_obj(*subvol_info);
>> +	if (!subvol_info)
>> +		return -ENOMEM;
>> +
>> +	subvol_info_32 = kzalloc_obj(*subvol_info_32);
>> +	if (!subvol_info_32) {
>> +		ret = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
>> +	if (ret)
>> +		goto out;
>> +
>> +	subvol_info_32->treeid = subvol_info->treeid;
>> +	memcpy(subvol_info_32->name, subvol_info->name, sizeof(subvol_info_32->name));
>> +	subvol_info_32->parent_id = subvol_info->parent_id;
>> +	subvol_info_32->dirid = subvol_info->dirid;
>> +	subvol_info_32->generation = subvol_info->generation;
>> +	subvol_info_32->flags = subvol_info->flags;
>> +	memcpy(subvol_info_32->uuid, subvol_info->uuid, BTRFS_UUID_SIZE);
>> +	memcpy(subvol_info_32->parent_uuid, subvol_info->parent_uuid, BTRFS_UUID_SIZE);
>> +	memcpy(subvol_info_32->received_uuid, subvol_info->received_uuid, BTRFS_UUID_SIZE);
>> +	subvol_info_32->ctransid = subvol_info->ctransid;
>> +	subvol_info_32->otransid = subvol_info->otransid;
>> +	subvol_info_32->stransid = subvol_info->stransid;
>> +	subvol_info_32->rtransid = subvol_info->rtransid;
>> +	subvol_info_32->ctime.sec = subvol_info->ctime.sec;
>> +	subvol_info_32->ctime.nsec = subvol_info->ctime.nsec;
>> +	subvol_info_32->otime.sec = subvol_info->otime.sec;
>> +	subvol_info_32->otime.nsec = subvol_info->otime.nsec;
>> +	subvol_info_32->stime.sec = subvol_info->stime.sec;
>> +	subvol_info_32->stime.nsec = subvol_info->stime.nsec;
>> +	subvol_info_32->rtime.sec = subvol_info->rtime.sec;
>> +	subvol_info_32->rtime.nsec = subvol_info->rtime.nsec;
>> +
>> +	if (copy_to_user(argp, subvol_info_32, sizeof(*subvol_info_32)))
>> +		ret = -EFAULT;
>> +
>> +out:
>> +	kfree(subvol_info_32);
>> +	kfree(subvol_info);
>> +	return ret;
>> +}
>> +#endif
>> +
>> +static int btrfs_ioctl_get_subvol_info(struct inode *inode, void __user *argp)
>> +{
>> +	struct btrfs_ioctl_get_subvol_info_args *subvol_info;
>> +	int ret;
>> +
>> +	subvol_info = kzalloc_obj(*subvol_info);
>> +	if (!subvol_info)
>> +		return -ENOMEM;
>> +
>> +	ret = _btrfs_ioctl_get_subvol_info(inode, subvol_info);
>> +	if (!ret && copy_to_user(argp, subvol_info, sizeof(*subvol_info)))
>> +		ret = -EFAULT;
>> +
>>  	kfree(subvol_info);
>>  	return ret;
>>  }
>> @@ -5273,6 +5355,10 @@ long btrfs_ioctl(struct file *file, unsigned int
>>  		return btrfs_ioctl_set_features(file, argp);
>>  	case BTRFS_IOC_GET_SUBVOL_INFO:
>>  		return btrfs_ioctl_get_subvol_info(inode, argp);
>> +#ifdef CONFIG_64BIT
>> +	case BTRFS_IOC_GET_SUBVOL_INFO_32:
>> +		return btrfs_ioctl_get_subvol_info_32(inode, argp);
>> +#endif
>>  	case BTRFS_IOC_GET_SUBVOL_ROOTREF:
>>  		return btrfs_ioctl_get_subvol_rootref(root, argp);
>>  	case BTRFS_IOC_INO_LOOKUP_USER:

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]