Re: [PATCH] btrfs: bugfix: handle FS_IOC32_{GETFLAGS,SETFLAGS,GETVERSION} in btrfs_ioctl
From: Thomas Rohwer
Date: Thu Oct 29 2015 - 08:11:11 EST
Hello,
I've investigated this now, and it seems to be the pointer-type clone_sources
member of struct btrfs_ioctl_send_args. I can't think of a perfect way to fix
this, but it might not be *too* ugly to:
- replace the current clone_sources with a u64 that must always be (u64)-1;
this causes older kernels to error cleanly if called with a new ioctl data
- use the top 1 or 2 bits of flags to indicate sizeof(void*) as it appears to
userspace OR just use up reserved[0] for pointer size:
io_send.ptr_size = sizeof(void*);
- replace one of the reserved fields with the new clone_sources
The way it was done for receive seems like it might not work for non-x86
compat interfaces (eg, MIPS n32) - but I could be wrong.
Thoughts?
I also encountered that problem with send. I posted the following a while ago to
comp.file-systems.btrfs, but never got any replies. The patch works for me (I used
it on my system in some cases), but is not extensively tested.
Sincerely,
Thomas Rohwer
Hello,
I am using as kernel Linux 4.1.3 (64bit) and btrfs-prog version 4.0 (32 bit user space).
I wanted to use send/receive with btrfs for the first time today and I got the following error:
humbur:~# btrfs send /snap > /dev/null
At subvol /snap
ERROR: send ioctl failed with -25: Inappropriate ioctl for device
ERROR: failed to read stream from kernel. Bad file descriptor
Investigating the source, I noticed that probably the problem is the member clone_sources in the structure
struct btrfs_ioctl_send_args {
__s64 send_fd; /* in */
__u64 clone_sources_count; /* in */
__u64 __user *clone_sources; /* in */
__u64 parent_root; /* in */
__u64 flags; /* in */
__u64 reserved[4]; /* in */
};
in include/uapi/linux/btrfs.h and the missing adaption code in fs/btrfs/ioctl.c. The member clone_sources is only 32 bit wide in case of 32 bit user space.
For the ioctl RECEIVED_SUBVOL somebody already added code for the in this case also necessary translation. I took this as a template and
wrote a patch (see below). The patch compiles and with the new kernel I seem to get valid data with send (I have to read it back yet,
but I get about the expected amount and structure).
This is a proof of concept patch; for example the compiler currently warns for
(args64->clone_sources = (__u64*)args32->clone_sources;
and I would have to investigate how to properly convert the pointer. Further there are probably some issues with the formating of the source code.
I also have not tested the 32bit/32bit 64bit/64bit userspace/kernel combinations.
If there is interest, I can resubmit an improved patch. Please CC me in replies, since I have not subscribed to the list.
Sincerely,
Thomas Rohwer
Signed-off-by: Thomas Rohwer <trohwer85@xxxxxxxxx>
From e4156c38105200fa83913b6a94f07a41631c5f75 Mon Sep 17 00:00:00 2001
From: Thomas <tr@xxxxxxxxxxxxxxxx>
Date: Wed, 22 Jul 2015 22:03:15 +0200
Subject: [PATCH] add 32 bit adaption code for send ioctl
---
fs/btrfs/ioctl.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/send.c | 11 +--------
fs/btrfs/send.h | 2 +-
3 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1c22c65..8b969c0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -83,6 +83,19 @@ 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_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
@@ -4965,6 +4978,43 @@ out:
kfree(args64);
return ret;
}
+
+static long btrfs_ioctl_send_32(struct file *file,
+ void __user *arg)
+{
+ struct btrfs_ioctl_send_args_32 *args32 = NULL;
+ struct btrfs_ioctl_send_args *args64 = NULL;
+ int ret = 0;
+
+ args32 = memdup_user(arg, sizeof(*args32));
+ if (IS_ERR(args32)) {
+ ret = PTR_ERR(args32);
+ args32 = NULL;
+ goto out;
+ }
+
+ args64 = kmalloc(sizeof(*args64), GFP_NOFS);
+ if (!args64) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ args64->send_fd = args32->send_fd;
+ args64->clone_sources_count = args32->clone_sources_count;
+ args64->clone_sources = (__u64*)args32->clone_sources;
+ args64->parent_root = args32->parent_root;
+ args64->flags = args32->flags;
+
+ ret = _btrfs_ioctl_send(file, args64);
+
+ // only in arguments, so no copy back to args32
+
+out:
+ kfree(args32);
+ kfree(args64);
+ return ret;
+}
+
#endif
static long btrfs_ioctl_set_received_subvol(struct file *file,
@@ -4994,6 +5044,27 @@ out:
return ret;
}
+static long btrfs_ioctl_send(struct file *file,
+ void __user *arg)
+{
+ struct btrfs_ioctl_send_args *sa = NULL;
+ int ret = 0;
+
+ sa = memdup_user(arg, sizeof(*sa));
+ if (IS_ERR(sa)) {
+ ret = PTR_ERR(sa);
+ sa = NULL;
+ goto out;
+ }
+
+ ret = _btrfs_ioctl_send(file, sa);
+
+out:
+ kfree(sa);
+ return ret;
+}
+
+
static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
{
struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
@@ -5320,6 +5391,8 @@ long btrfs_ioctl(struct file *file, unsigned int
#ifdef CONFIG_64BIT
case BTRFS_IOC_SET_RECEIVED_SUBVOL_32:
return btrfs_ioctl_set_received_subvol_32(file, argp);
+ case BTRFS_IOC_SEND_32:
+ return btrfs_ioctl_send_32(file, argp);
#endif
case BTRFS_IOC_SEND:
return btrfs_ioctl_send(file, argp);
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index a1216f9..6838078 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5663,13 +5663,12 @@ 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, struct btrfs_ioctl_send_args *arg)
{
int ret = 0;
struct btrfs_root *send_root;
struct btrfs_root *clone_root;
struct btrfs_fs_info *fs_info;
- struct btrfs_ioctl_send_args *arg = NULL;
struct btrfs_key key;
struct send_ctx *sctx = NULL;
u32 i;
@@ -5707,13 +5706,6 @@ 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 (!access_ok(VERIFY_READ, arg->clone_sources,
sizeof(*arg->clone_sources) *
arg->clone_sources_count)) {
@@ -5942,7 +5934,6 @@ out:
if (sctx && !IS_ERR_OR_NULL(sctx->parent_root))
btrfs_root_dec_send_in_progress(sctx->parent_root);
- kfree(arg);
vfree(clone_sources_tmp);
if (sctx) {
diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
index 48d425a..fb43556 100644
--- a/fs/btrfs/send.h
+++ b/fs/btrfs/send.h
@@ -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, struct btrfs_ioctl_send_args *arg);
#endif
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/