mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Thomas Rohwer <tr@ohwer.de>
Cc: Luke Dashjr <luke@dashjr.org>,
	dsterba@suse.cz, Chris Mason <clm@fb.com>,
	Josef Bacik <jbacik@fb.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] btrfs: bugfix: handle FS_IOC32_{GETFLAGS,SETFLAGS,GETVERSION} in btrfs_ioctl
Date: Thu, 29 Oct 2015 16:25:36 +0100	[thread overview]
Message-ID: <20151029152536.GA13624@twin.jikos.cz> (raw)
In-Reply-To: <56320B79.9090207@gmail.com>

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.

  reply	other threads:[~2015-10-29 15:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 17:15 Luke Dashjr
2015-05-13 17:38 ` Greg KH
2015-05-14 14:06 ` David Sterba
2015-05-14 16:27   ` Luke Dashjr
2015-05-15 11:19     ` David Sterba
2015-05-15 16:35       ` Luke Dashjr
2015-10-29  8:22       ` Luke Dashjr
2015-10-29 12:05         ` Thomas Rohwer
2015-10-29 15:25           ` David Sterba [this message]
2015-10-29 14:39         ` David Sterba
2015-10-29 19:01           ` Luke Dashjr
2015-10-29 19:36           ` Thomas Rohwer
2015-10-29 20:04             ` Luke Dashjr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151029152536.GA13624@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=clm@fb.com \
    --cc=jbacik@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luke@dashjr.org \
    --cc=tr@ohwer.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®