* Re: [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO
2026-05-21 7:51 [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO Daan De Meyer
@ 2026-06-17 10:44 ` Qu Wenruo
2026-06-17 11:18 ` Miquel Sabaté Solà
1 sibling, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-06-17 10:44 UTC (permalink / raw)
To: Daan De Meyer, linux-btrfs; +Cc: linux-kernel
在 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:
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO
2026-05-21 7:51 [PATCH] btrfs: add 32-bit compat ioctl for BTRFS_IOC_GET_SUBVOL_INFO Daan De Meyer
2026-06-17 10:44 ` Qu Wenruo
@ 2026-06-17 11:18 ` Miquel Sabaté Solà
2026-06-17 11:22 ` Miquel Sabaté Solà
1 sibling, 1 reply; 4+ messages in thread
From: Miquel Sabaté Solà @ 2026-06-17 11:18 UTC (permalink / raw)
To: Daan De Meyer; +Cc: linux-btrfs, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 4+ messages in thread