btrfs: fix send ioctl on 32bit with 64bit kernel

PATCH Superseded Josef Bacik 2 participants 3 messages Sep 26, 2017 – Sep 27, 2017 Btrfs file system

Share & tools

Josef Bacik

author

2017-09-26 20:40 UTC #0 lore raw

PATCH 1/1 btrfs: fix send ioctl on 32bit with 64bit kernel

We pass in a pointer in our send arg struct, this means the struct size doesn't match with 32bit user space and 64bit kernel space. Fix this by adding a compat mode and doing the appropriate conversion.

Signed-off-by Josef Bacik <[email protected]>

---
 fs/btrfs/ioctl.c           |  6 +++++-
 fs/btrfs/send.c            | 34 ++++++++++++++++++++++++++++------
 fs/btrfs/send.h            |  2 +-
 include/uapi/linux/btrfs.h | 13 +++++++++++++
 4 files changed, 47 insertions(+), 8 deletions(-)

fs/btrfs/ioctl.c +5 -1

index 050d2d9c5533..9169d67e49b9 100644
@@ -5594,7 +5594,11 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_set_received_subvol_32(file, argp);
 #endif
 	case BTRFS_IOC_SEND:
-		return btrfs_ioctl_send(file, argp);
+		return btrfs_ioctl_send(file, argp, false);
+#ifdef CONFIG_64BIT
+	case BTRFS_IOC_SEND_32:
+		return btrfs_ioctl_send(file, argp, true);
+#endif
 	case BTRFS_IOC_GET_DEV_STATS:
 		return btrfs_ioctl_get_dev_stats(fs_info, argp);
 	case BTRFS_IOC_QUOTA_CTL:

fs/btrfs/send.c +28 -6

index 32b043ef8ac9..2041cac1875a 100644
@@ -26,6 +26,7 @@
 #include <linux/radix-tree.h>
 #include <linux/vmalloc.h>
 #include <linux/string.h>
+#include <linux/compat.h>
 
 #include "send.h"
 #include "backref.h"

@@ -6365,7 +6366,7 @@ static void btrfs_root_dec_send_in_progress(struct btrfs_root* root)
 	spin_unlock(&root->root_item_lock);
 }
 
-long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
+long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_, bool compat)
 {
 	int ret = 0;
 	struct btrfs_root *send_root = BTRFS_I(file_inode(mnt_file))->root;

@@ -6407,11 +6408,32 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
 		goto out;
 	}
 
-	arg = memdup_user(arg_, sizeof(*arg));
-	if (IS_ERR(arg)) {
-		ret = PTR_ERR(arg);
-		arg = NULL;
-		goto out;
+	if (compat) {
+		struct btrfs_ioctl_send_args_32 args32;
+		ret = copy_from_user(&args32, arg_, sizeof(args32));
+		if (ret) {
+			btrfs_err(fs_info, "args32 copy failed\n");
+			goto out;
+		}
+		arg = kzalloc(sizeof(*arg), GFP_KERNEL);
+		if (!arg) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		arg->send_fd = args32.send_fd;
+		arg->clone_sources_count = args32.clone_sources_count;
+		arg->clone_sources = compat_ptr(args32.clone_sources);
+		arg->parent_root = args32.parent_root;
+		arg->flags = args32.flags;
+		memcpy(arg->reserved, args32.reserved,
+		       sizeof(args32.reserved));
+	} else {
+		arg = memdup_user(arg_, sizeof(*arg));
+		if (IS_ERR(arg)) {
+			ret = PTR_ERR(arg);
+			arg = NULL;
+			goto out;
+		}
 	}
 
 	/*

fs/btrfs/send.h +1 -1

index 02e00166c4da..d45d2471c4b6 100644
@@ -130,5 +130,5 @@ enum {
 #define BTRFS_SEND_A_MAX (__BTRFS_SEND_A_MAX - 1)
 
 #ifdef __KERNEL__
-long btrfs_ioctl_send(struct file *mnt_file, void __user *arg);
+long btrfs_ioctl_send(struct file *mnt_file, void __user *arg, bool compat);
 #endif

include/uapi/linux/btrfs.h +13 -0

index 378230c163d5..50b201222cfc 100644
@@ -719,6 +719,19 @@ struct btrfs_ioctl_send_args {
 	__u64 reserved[4];		/* in */
 };
 
+#ifdef CONFIG_64BIT
+struct btrfs_ioctl_send_args_32 {
+	__s64 send_fd;			/* in */
+	__u64 clone_sources_count;	/* in */
+	__u32 clone_sources;		/* in */
+	__u64 parent_root;		/* in */
+	__u64 flags;			/* in */
+	__u64 reserved[4];		/* in */
+} __attribute__ ((__packed__));
+#define BTRFS_IOC_SEND_32 _IOW(BTRFS_IOCTL_MAGIC, 38, \
+			       struct btrfs_ioctl_send_args_32)
+#endif
+
 /* Error codes as returned by the kernel */
 enum btrfs_err_code {
 	BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET = 1,

Signature · 1 line

2.7.4

David Sterba

2017-09-27 10:54 UTC #1 lore raw

Quoted 3 lines · Josef Bacik

We pass in a pointer in our send arg struct, this means the struct size doesn't match with 32bit user space and 64bit kernel space. Fix this by adding a compat mode and doing the appropriate conversion.

How does this differ from the existing workaround that adds the 32bit structure (btrfs_ioctl_timespec_32 and btrfs_ioctl_received_subvol_args_32 at the top of ioctl.c) and the btrfs_compat_ioctl() (at the end of ioctl.c)?

Josef Bacik

author

2017-09-27 14:41 UTC #2 lore raw

Quoted 9 lines · David Sterba

Quoted 3 lines · Josef Bacik
We pass in a pointer in our send arg struct, this means the struct size doesn't match with 32bit user space and 64bit kernel space. Fix this by adding a compat mode and doing the appropriate conversion.
How does this differ from the existing workaround that adds the 32bit structure (btrfs_ioctl_timespec_32 and btrfs_ioctl_received_subvol_args_32 at the top of ioctl.c) and the btrfs_compat_ioctl() (at the end of ioctl.c)?

This is different because it's for SEND not RECIEVED_SUBVOL. We have a pointer in the send args struct which is differently sized on 64bit from 32bit, so we need the compat struct so it's the right size when it comes into the kernel, and then we translate from there. Without this send is broken with 32bit userspace and a 64bit kernel. Thanks,

Josef