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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151029152536.GA13624@twin.jikos.cz>
Date:	Thu, 29 Oct 2015 16:25:36 +0100
From:	David Sterba <dsterba@...e.cz>
To:	Thomas Rohwer <tr@...er.de>
Cc:	Luke Dashjr <luke@...hjr.org>, dsterba@...e.cz,
	Chris Mason <clm@...com>, Josef Bacik <jbacik@...com>,
	linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: bugfix: handle
 FS_IOC32_{GETFLAGS,SETFLAGS,GETVERSION} in btrfs_ioctl

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/