Re: [PATCH] btrfs: bugfix: handle FS_IOC32_{GETFLAGS,SETFLAGS,GETVERSION} in btrfs_ioctl

David Sterba
2015-10-29T15:27:06Z

On Thu, Oct 29, 2015 at 01:05:13PM +0100, Thomas Rohwer wrote:
> Investigating the source, I noticed that probably the problem is the member clone_sources in the structure

That's, right. Yet another thing to keep in mind when designing ioctls.

> 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).

I'm not yet sure if this is the right approach, but I'm still
considering it. My suggestion is to add the union and force the width.
In case of the receive subvol this was not possible because it was
around an external structure.

The basic difference if we use the same structure definition, let the
compiler pick the pointer type width, and do the conversion in kernel
transparently (ie. the RECEIVED_SUBVOL workaround), or if we patch the
structure so the pointer magic happens in the userspace.

> 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.

args64->clone_sources = (__u64*)(long)args32->clone_sources;

should work.

> Further there are probably some issues with the formating of the source code.

Yeah there are some, but at this point we're not sure what's the right
approach so don't worry.