[PATCH v5 00/18] compat_ioctl.c removal, part 2/3

[PATCH v5 00/18] compat_ioctl.c removal, part 2/3

[Arnd Bergmann]

This is a follow-up to part 1/3 that I posted after -rc2.
I hope these are still largely uncontroversial changes, and
I would like to get them into linux-5.4.

Part 1 was in

https://lore.kernel.org/lkml/CAPcyv4i_nHzV155RcgnAQ189aq2Lfd2g8pA1D5NbZqo9E_u+Dw@mail.gmail.com/

Part 3 will be one kernel release after part 2 is merged,
as that still needs a little extra work.

The entire series is available at

git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git compat_ioctl

      Arnd

Al Viro (2):
  compat_ioctl: unify copy-in of ppp filters
  compat_ioctl: move PPPIOCSCOMPRESS to ppp_generic

Arnd Bergmann (16):
  xfs: compat_ioctl: use compat_ptr()
  xfs: compat_ioctl: add missing conversions
  gfs2: add compat_ioctl support
  fs: compat_ioctl: move FITRIM emulation into file systems
  watchdog: cpwd: use generic compat_ptr_ioctl
  compat_ioctl: move WDIOC handling into wdt drivers
  compat_ioctl: reimplement SG_IO handling
  af_unix: add compat_ioctl support
  compat_ioctl: handle SIOCOUTQNSD
  compat_ioctl: move SIOCOUTQ out of compat_ioctl.c
  tty: handle compat PPP ioctls
  compat_ioctl: handle PPPIOCGIDLE for 64-bit time_t
  compat_ioctl: ppp: move simple commands into ppp_generic.c
  compat_ioctl: move SG_GET_REQUEST_TABLE handling
  pktcdvd: add compat_ioctl handler
  scsi: sd: enable compat ioctls for sed-opal

 Documentation/networking/ppp_generic.txt  |   2 +
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c |   1 +
 arch/um/drivers/harddog_kern.c            |   1 +
 block/scsi_ioctl.c                        | 132 ++++++++-
 drivers/block/pktcdvd.c                   |  25 ++
 drivers/char/ipmi/ipmi_watchdog.c         |   1 +
 drivers/hwmon/fschmd.c                    |   1 +
 drivers/net/ppp/ppp_generic.c             | 245 ++++++++++-----
 drivers/rtc/rtc-ds1374.c                  |   1 +
 drivers/scsi/sd.c                         |  14 +-
 drivers/scsi/sg.c                         |  59 +++-
 drivers/tty/tty_io.c                      |   5 +
 drivers/watchdog/acquirewdt.c             |   1 +
 drivers/watchdog/advantechwdt.c           |   1 +
 drivers/watchdog/alim1535_wdt.c           |   1 +
 drivers/watchdog/alim7101_wdt.c           |   1 +
 drivers/watchdog/ar7_wdt.c                |   1 +
 drivers/watchdog/at91rm9200_wdt.c         |   1 +
 drivers/watchdog/ath79_wdt.c              |   1 +
 drivers/watchdog/bcm63xx_wdt.c            |   1 +
 drivers/watchdog/cpu5wdt.c                |   1 +
 drivers/watchdog/cpwd.c                   |  25 +-
 drivers/watchdog/eurotechwdt.c            |   1 +
 drivers/watchdog/f71808e_wdt.c            |   1 +
 drivers/watchdog/gef_wdt.c                |   1 +
 drivers/watchdog/geodewdt.c               |   1 +
 drivers/watchdog/ib700wdt.c               |   1 +
 drivers/watchdog/ibmasr.c                 |   1 +
 drivers/watchdog/indydog.c                |   1 +
 drivers/watchdog/intel_scu_watchdog.c     |   1 +
 drivers/watchdog/iop_wdt.c                |   1 +
 drivers/watchdog/it8712f_wdt.c            |   1 +
 drivers/watchdog/ixp4xx_wdt.c             |   1 +
 drivers/watchdog/ks8695_wdt.c             |   1 +
 drivers/watchdog/m54xx_wdt.c              |   1 +
 drivers/watchdog/machzwd.c                |   1 +
 drivers/watchdog/mixcomwd.c               |   1 +
 drivers/watchdog/mtx-1_wdt.c              |   1 +
 drivers/watchdog/mv64x60_wdt.c            |   1 +
 drivers/watchdog/nuc900_wdt.c             |   1 +
 drivers/watchdog/nv_tco.c                 |   1 +
 drivers/watchdog/pc87413_wdt.c            |   1 +
 drivers/watchdog/pcwd.c                   |   1 +
 drivers/watchdog/pcwd_pci.c               |   1 +
 drivers/watchdog/pcwd_usb.c               |   1 +
 drivers/watchdog/pika_wdt.c               |   1 +
 drivers/watchdog/pnx833x_wdt.c            |   1 +
 drivers/watchdog/rc32434_wdt.c            |   1 +
 drivers/watchdog/rdc321x_wdt.c            |   1 +
 drivers/watchdog/riowd.c                  |   1 +
 drivers/watchdog/sa1100_wdt.c             |   1 +
 drivers/watchdog/sb_wdog.c                |   1 +
 drivers/watchdog/sbc60xxwdt.c             |   1 +
 drivers/watchdog/sbc7240_wdt.c            |   1 +
 drivers/watchdog/sbc_epx_c3.c             |   1 +
 drivers/watchdog/sbc_fitpc2_wdt.c         |   1 +
 drivers/watchdog/sc1200wdt.c              |   1 +
 drivers/watchdog/sc520_wdt.c              |   1 +
 drivers/watchdog/sch311x_wdt.c            |   1 +
 drivers/watchdog/scx200_wdt.c             |   1 +
 drivers/watchdog/smsc37b787_wdt.c         |   1 +
 drivers/watchdog/w83877f_wdt.c            |   1 +
 drivers/watchdog/w83977f_wdt.c            |   1 +
 drivers/watchdog/wafer5823wdt.c           |   1 +
 drivers/watchdog/watchdog_dev.c           |   1 +
 drivers/watchdog/wdrtas.c                 |   1 +
 drivers/watchdog/wdt.c                    |   1 +
 drivers/watchdog/wdt285.c                 |   1 +
 drivers/watchdog/wdt977.c                 |   1 +
 drivers/watchdog/wdt_pci.c                |   1 +
 fs/compat_ioctl.c                         | 346 +---------------------
 fs/ecryptfs/file.c                        |   1 +
 fs/ext4/ioctl.c                           |   1 +
 fs/f2fs/file.c                            |   1 +
 fs/gfs2/file.c                            |  24 ++
 fs/hpfs/dir.c                             |   1 +
 fs/hpfs/file.c                            |   1 +
 fs/nilfs2/ioctl.c                         |   1 +
 fs/ocfs2/ioctl.c                          |   1 +
 fs/xfs/xfs_ioctl32.c                      |  11 +-
 include/linux/blkdev.h                    |   2 +
 include/uapi/linux/ppp-ioctl.h            |   2 +
 include/uapi/linux/ppp_defs.h             |  14 +
 lib/iov_iter.c                            |   1 +
 net/socket.c                              |   3 +
 net/unix/af_unix.c                        |  19 ++
 86 files changed, 526 insertions(+), 472 deletions(-)

-- 
2.20.0

Cc: davem@davemloft.net
Cc: axboe@kernel.dk
Cc: linux-block@vger.kernel.org
Cc: minyard@acm.org
Cc: gregkh@linuxfoundation.org
Cc: linux@roeck-us.net
Cc: alexandre.belloni@bootlin.com
Cc: jejb@linux.ibm.com
Cc: martin.petersen@oracle.com
Cc: dgilbert@interlog.com
Cc: jslaby@suse.com
Cc: wim@linux-watchdog.org
Cc: viro@zeniv.linux.org.uk
Cc: tytso@mit.edu
Cc: adilger.kernel@dilger.ca
Cc: jaegeuk@kernel.org
Cc: rpeterso@redhat.com
Cc: agruenba@redhat.com
Cc: mikulas@artax.karlin.mff.cuni.cz
Cc: konishi.ryusuke@gmail.com
Cc: jlbec@evilplan.org
Cc: joseph.qi@linux.alibaba.com
Cc: darrick.wong@oracle.com
Cc: linux-xfs@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: openipmi-developer@lists.sourceforge.net
Cc: linux-hwmon@vger.kernel.org
Cc: linux-ppp@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: ecryptfs@vger.kernel.org
Cc: linux-ext4@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: cluster-devel@redhat.com
Cc: linux-nilfs@vger.kernel.org
Cc: ocfs2-devel@oss.oracle.com

[PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

For 31-bit s390 user space, we have to pass pointer arguments through
compat_ptr() in the compat_ioctl handler.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/xfs/xfs_ioctl32.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 7fcf7569743f..ad91e81a2fcf 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -547,7 +547,7 @@ xfs_file_compat_ioctl(
 	struct inode		*inode = file_inode(filp);
 	struct xfs_inode	*ip = XFS_I(inode);
 	struct xfs_mount	*mp = ip->i_mount;
-	void			__user *arg = (void __user *)p;
+	void			__user *arg = compat_ptr(p);
 	int			error;
 
 	trace_xfs_file_compat_ioctl(ip);
@@ -576,7 +576,7 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_SCRUB_METADATA:
 	case XFS_IOC_BULKSTAT:
 	case XFS_IOC_INUMBERS:
-		return xfs_file_ioctl(filp, cmd, p);
+		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
 #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
 	/*
 	 * These are handled fine if no alignment issues.  To support x32
@@ -602,7 +602,7 @@ xfs_file_compat_ioctl(
 	 */
 	case XFS_IOC_SWAPEXT:
 #endif
-		return xfs_file_ioctl(filp, cmd, p);
+		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
 #endif
 #if defined(BROKEN_X86_ALIGNMENT)
 	case XFS_IOC_ALLOCSP_32:
@@ -653,7 +653,7 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_SETXFLAGS_32:
 	case XFS_IOC_GETVERSION_32:
 		cmd = _NATIVE_IOC(cmd, long);
-		return xfs_file_ioctl(filp, cmd, p);
+		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
 	case XFS_IOC_SWAPEXT_32: {
 		struct xfs_swapext	  sxp;
 		struct compat_xfs_swapext __user *sxu = arg;
-- 
2.20.0

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Dave Chinner]

On Wed, Aug 14, 2019 at 10:42:28PM +0200, Arnd Bergmann wrote:
> For 31-bit s390 user space, we have to pass pointer arguments through
> compat_ptr() in the compat_ioctl handler.

Seems fair enough, but...
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  fs/xfs/xfs_ioctl32.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 7fcf7569743f..ad91e81a2fcf 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -547,7 +547,7 @@ xfs_file_compat_ioctl(
>  	struct inode		*inode = file_inode(filp);
>  	struct xfs_inode	*ip = XFS_I(inode);
>  	struct xfs_mount	*mp = ip->i_mount;
> -	void			__user *arg = (void __user *)p;
> +	void			__user *arg = compat_ptr(p);
>  	int			error;
>  
>  	trace_xfs_file_compat_ioctl(ip);
> @@ -576,7 +576,7 @@ xfs_file_compat_ioctl(
>  	case XFS_IOC_SCRUB_METADATA:
>  	case XFS_IOC_BULKSTAT:
>  	case XFS_IOC_INUMBERS:
> -		return xfs_file_ioctl(filp, cmd, p);
> +		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);

I don't really like having to sprinkle special casts through the
code because of this.

Perhaps do something like:

static inline unsigned long compat_ptr_mask(unsigned long p)
{
	return (unsigned long)compat_ptr(p);
}

and then up front you can do:

	void	__user *arg;

	p = compat_ptr_mask(p);
	arg = (void __user *)p;


and then the rest of the code remains unchanged by now uses p
correctly instead of having to change all the code to cast arg back
to an unsigned long...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Wed, Aug 14, 2019 at 11:39 PM Dave Chinner <david@fromorbit.com> wrote:
> >       case XFS_IOC_BULKSTAT:
> >       case XFS_IOC_INUMBERS:
> > -             return xfs_file_ioctl(filp, cmd, p);
> > +             return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
>
> I don't really like having to sprinkle special casts through the
> code because of this.
>
> Perhaps do something like:
>
> static inline unsigned long compat_ptr_mask(unsigned long p)
> {
>         return (unsigned long)compat_ptr(p);
> }
>
> and then up front you can do:
>
>         void    __user *arg;
>
>         p = compat_ptr_mask(p);
>         arg = (void __user *)p;
>
>
> and then the rest of the code remains unchanged by now uses p
> correctly instead of having to change all the code to cast arg back
> to an unsigned long...
>

In part 1 of the series, I define this function as a global:

long compat_ptr_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
        if (!file->f_op->unlocked_ioctl)
                return -ENOIOCTLCMD;

        return file->f_op->unlocked_ioctl(file, cmd, (unsigned
long)compat_ptr(arg));
}

How about using that to replace the individual casts:

-       return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
+      return compat_ptr_ioctl(filp, cmd, arg);

It adds another indirection, but it avoids all the casts and
uses existing mechanism.

     Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Christoph Hellwig]

On Thu, Aug 15, 2019 at 07:37:53AM +1000, Dave Chinner wrote:
> > @@ -576,7 +576,7 @@ xfs_file_compat_ioctl(
> >  	case XFS_IOC_SCRUB_METADATA:
> >  	case XFS_IOC_BULKSTAT:
> >  	case XFS_IOC_INUMBERS:
> > -		return xfs_file_ioctl(filp, cmd, p);
> > +		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
> 
> I don't really like having to sprinkle special casts through the
> code because of this.

True.  But the proper fix is to not do the indirection through
xfs_file_ioctl but instead to call xfs_ioc_scrub_metadata,
xfs_ioc_bulkstat, etc directly which all take a void __user
arguments already.

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Thu, Aug 15, 2019 at 9:13 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Aug 15, 2019 at 07:37:53AM +1000, Dave Chinner wrote:
> > > @@ -576,7 +576,7 @@ xfs_file_compat_ioctl(
> > >     case XFS_IOC_SCRUB_METADATA:
> > >     case XFS_IOC_BULKSTAT:
> > >     case XFS_IOC_INUMBERS:
> > > -           return xfs_file_ioctl(filp, cmd, p);
> > > +           return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
> >
> > I don't really like having to sprinkle special casts through the
> > code because of this.
>
> True.  But the proper fix is to not do the indirection through
> xfs_file_ioctl but instead to call xfs_ioc_scrub_metadata,
> xfs_ioc_bulkstat, etc directly which all take a void __user
> arguments already.

I'm not sure that's better: This would end up duplicating all
of xfs_file_ioctl(), which is already a fairly long function, compared
to the current way of having a large set of commands all handled
with a single line.

>From looking at other subsystems, what I find to work best is to
move the compat handler into the same file as the native code
and then structure the files so that shared handlers get
put into one place, something like

/* these are the ones that have the same ABI for 32-bit and 64-bit tasks */
static int xfs_compatible_file_ioctl(struct file *filp, unsigned cmd,
void __user *p)
{
      int ret = -ENOIOCTLCMD;

       switch (cmd) {
       case XFS_IOC_DIOINFO:
            ...
        case ...
     }

     return ret;
}

long
xfs_file_compat_ioctl(
        struct file             *filp,
        unsigned                cmd,
        unsigned long           p)
{
       ret = xfs_compatible_file_ioctl(filp, cmd, compat_ptr(p));
       if (ret != -ENOIOCTLCMD)
              return ret;

      /* all incompatible ones below */
      switch (cmd) {
         ...
      }
}
Having them in one place makes it more obvious to readers how the
native and compat handlers fit together, and makes it easier to keep
the two in sync.

That would of course be a much larger change to how it's done today,
and it's way out of scope of what I want to achieve in my (already
too long) series.

     Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Thu, Aug 15, 2019 at 9:13 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Aug 15, 2019 at 07:37:53AM +1000, Dave Chinner wrote:
> > > @@ -576,7 +576,7 @@ xfs_file_compat_ioctl(
> > >     case XFS_IOC_SCRUB_METADATA:
> > >     case XFS_IOC_BULKSTAT:
> > >     case XFS_IOC_INUMBERS:
> > > -           return xfs_file_ioctl(filp, cmd, p);
> > > +           return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
> >
> > I don't really like having to sprinkle special casts through the
> > code because of this.
>
> True.  But the proper fix is to not do the indirection through
> xfs_file_ioctl but instead to call xfs_ioc_scrub_metadata,
> xfs_ioc_bulkstat, etc directly which all take a void __user
> arguments already.

I'm not sure that's better: This would end up duplicating all
of xfs_file_ioctl(), which is already a fairly long function, compared
to the current way of having a large set of commands all handled
with a single line.

From looking at other subsystems, what I find to work best is to
move the compat handler into the same file as the native code
and then structure the files so that shared handlers get
put into one place, something like

/* these are the ones that have the same ABI for 32-bit and 64-bit tasks */
static int xfs_compatible_file_ioctl(struct file *filp, unsigned cmd,
void __user *p)
{
      int ret = -ENOIOCTLCMD;

       switch (cmd) {
       case XFS_IOC_DIOINFO:
            ...
        case ...
     }

     return ret;
}

long
xfs_file_compat_ioctl(
        struct file             *filp,
        unsigned                cmd,
        unsigned long           p)
{
       ret = xfs_compatible_file_ioctl(filp, cmd, compat_ptr(p));
       if (ret != -ENOIOCTLCMD)
              return ret;

      /* all incompatible ones below */
      switch (cmd) {
         ...
      }
}
Having them in one place makes it more obvious to readers how the
native and compat handlers fit together, and makes it easier to keep
the two in sync.

That would of course be a much larger change to how it's done today,
and it's way out of scope of what I want to achieve in my (already
too long) series.

     Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Christoph Hellwig]

In many ways I'd actually much rather have a table driven approach.
Let me try something..

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Christoph Hellwig]

On Thu, Aug 15, 2019 at 01:02:11AM -0700, Christoph Hellwig wrote:
> In many ways I'd actually much rather have a table driven approach.
> Let me try something..

Ok, it seems like we don't even need a table containing native and
compat as we can just fall back.  The tables still seem nicer to read,
though.

Let me know what you think of this:

http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table

I also wonder if we should life the ioctl handler tables to the
VFS..

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Thu, Aug 15, 2019 at 12:26 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Aug 15, 2019 at 01:02:11AM -0700, Christoph Hellwig wrote:
> > In many ways I'd actually much rather have a table driven approach.
> > Let me try something..
>
> Ok, it seems like we don't even need a table containing native and
> compat as we can just fall back.  The tables still seem nicer to read,
> though.
>
> Let me know what you think of this:
>
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table

These all look like useful cleanups, but I'm a little worried about introducing
merge conflicts with my own patches. I would want to have my series get
merged as a complete branch since each patch that removes a bit of
fs/compat_ioctl.c would clash with a patch removing the adjacent bits
otherwise.

I still haven't heard from Al regarding what he thinks of my v5 series.
If he wants me to send a pull request for it, I can of course add in
your patches  after they are fully reviewed.

> I also wonder if we should life the ioctl handler tables to the VFS.

The idea of these tables has come up a few times in the past,
and there are a couple of subsystems that have something like it,
e.g. drivers/media.

Usually you'd want to combine the table with a more generic way to
do the copy_from_user()/copy_to_user() on the argument, but that
in turn requires all commands to be defined correctly (a lot of drivers
have some commands that specify the wrong direction or the wrong
size, or one that predates the _IO() macro).

What I could imaging having in the long run is to have the ioctl table
attached to the file_operations structure, and then define it in a way
that handles at least the more common variations:

- copy_from_user to stack, pass a kernel pointer to handler
- a single entry for commands that are 32/64-bit compatible
- entries that are only used for native vs compat mode if they
  have incompatible arguments (this could also be handled
  by calling in_compat_syscall() in the handler itself).
- a flag to specify handlers that require the __user pointer instead
  of the implied copy.

Doing this right will certainly require several revisions of patch
series and lots of discussions, and is unrelated to the removal
of fs/compat_ioctl.c, so I'd much prefer to get this series merged
before we start working on that.

       Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Dave Chinner]

On Thu, Aug 15, 2019 at 03:26:49AM -0700, Christoph Hellwig wrote:
> On Thu, Aug 15, 2019 at 01:02:11AM -0700, Christoph Hellwig wrote:
> > In many ways I'd actually much rather have a table driven approach.
> > Let me try something..
> 
> Ok, it seems like we don't even need a table containing native and
> compat as we can just fall back.  The tables still seem nicer to read,
> though.
> 
> Let me know what you think of this:
> 
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table

Lots to like in that handful of patches. :)

It can easily go before or after Arnd's patch, and the merge
conflict either way would be minor, so I'm not really fussed either
way this gets sorted out...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Christoph Hellwig]

On Thu, Aug 15, 2019 at 10:15:12PM +1000, Dave Chinner wrote:
> > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table
> 
> Lots to like in that handful of patches. :)
> 
> It can easily go before or after Arnd's patch, and the merge
> conflict either way would be minor, so I'm not really fussed either
> way this gets sorted out...

The other thing we could do is to just pick the two important ones:

http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table-5.3

and throw that into Arnds series, or even 5.3, and then defer the
table thing until later.

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Thu, Aug 15, 2019 at 4:04 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Thu, Aug 15, 2019 at 10:15:12PM +1000, Dave Chinner wrote:
> > > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table
> >
> > Lots to like in that handful of patches. :)
> >
> > It can easily go before or after Arnd's patch, and the merge
> > conflict either way would be minor, so I'm not really fussed either
> > way this gets sorted out...
>
> The other thing we could do is to just pick the two important ones:
>
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table-5.3
>
> and throw that into Arnds series, or even 5.3, and then defer the
> table thing until later.

If we can have your "xfs: fall back to native ioctls for unhandled compat
ones" in 5.3, that would be ideal from my side, then I can just drop the
corresponding patch from my series and have the rest merged for 5.4.

The compat_ptr addition is independent of my series, I just added it
because I noticed it was missing, so we can merged that through
the xfs tree along with your other changes, either for 5.3 or 5.4.

     Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Darrick J. Wong]

On Thu, Aug 15, 2019 at 09:20:32PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 15, 2019 at 4:04 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Thu, Aug 15, 2019 at 10:15:12PM +1000, Dave Chinner wrote:
> > > > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table
> > >
> > > Lots to like in that handful of patches. :)
> > >
> > > It can easily go before or after Arnd's patch, and the merge
> > > conflict either way would be minor, so I'm not really fussed either
> > > way this gets sorted out...
> >
> > The other thing we could do is to just pick the two important ones:
> >
> > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table-5.3
> >
> > and throw that into Arnds series, or even 5.3, and then defer the
> > table thing until later.
> 
> If we can have your "xfs: fall back to native ioctls for unhandled compat
> ones" in 5.3, that would be ideal from my side, then I can just drop the
> corresponding patch from my series and have the rest merged for 5.4.
> 
> The compat_ptr addition is independent of my series, I just added it
> because I noticed it was missing, so we can merged that through
> the xfs tree along with your other changes, either for 5.3 or 5.4.

Er... do the two patches in the -5.3 branch actually fix something
that's broken?  I sense s390 is missing a pointer sanitization check or
something...?

--D

>      Arnd

Re: [PATCH v5 01/18] xfs: compat_ioctl: use compat_ptr()

[Arnd Bergmann]

On Thu, Aug 15, 2019 at 9:28 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Thu, Aug 15, 2019 at 09:20:32PM +0200, Arnd Bergmann wrote:
> > On Thu, Aug 15, 2019 at 4:04 PM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Thu, Aug 15, 2019 at 10:15:12PM +1000, Dave Chinner wrote:
> > > > > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table
> > > >
> > > > Lots to like in that handful of patches. :)
> > > >
> > > > It can easily go before or after Arnd's patch, and the merge
> > > > conflict either way would be minor, so I'm not really fussed either
> > > > way this gets sorted out...
> > >
> > > The other thing we could do is to just pick the two important ones:
> > >
> > > http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-ioctl-table-5.3
> > >
> > > and throw that into Arnds series, or even 5.3, and then defer the
> > > table thing until later.
> >
> > If we can have your "xfs: fall back to native ioctls for unhandled compat
> > ones" in 5.3, that would be ideal from my side, then I can just drop the
> > corresponding patch from my series and have the rest merged for 5.4.
> >
> > The compat_ptr addition is independent of my series, I just added it
> > because I noticed it was missing, so we can merged that through
> > the xfs tree along with your other changes, either for 5.3 or 5.4.
>
> Er... do the two patches in the -5.3 branch actually fix something
> that's broken?  I sense s390 is missing a pointer sanitization check or
> something...?

s390 is indeed missing the pointer conversion, the other patch
adds compat ioctl support for FS_IOC_GETFSLABEL and
FS_IOC_SETFSLABEL, which were missing, and it ensures that
FITRIM keeps working after I remove it from the list in
fs/compat_ioctl.c

       Arnd

[PATCH v5 02/18] xfs: compat_ioctl: add missing conversions

[Arnd Bergmann]

FS_IOC_GETFSLABEL/FS_IOC_SETFSLABEL were added in linux-4.18
in xfs, but not in the compat_ioctl case, so add them here.

FITRIM was added earlier and also lacks a line the same function,
but this is ok because there is an entry in fs/compat_ioctl.c for
it.

Adding all three here to keep the native and compat ioctl handlers
in sync.

Cc: stable@vger.kernel.org
Fixes: f7664b31975b ("xfs: implement online get/set fs label")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/xfs/xfs_ioctl32.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index ad91e81a2fcf..63bdc4c1535b 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -576,6 +576,9 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_SCRUB_METADATA:
 	case XFS_IOC_BULKSTAT:
 	case XFS_IOC_INUMBERS:
+	case FITRIM:
+	case FS_IOC_GETFSLABEL:
+	case FS_IOC_SETFSLABEL:
 		return xfs_file_ioctl(filp, cmd, (unsigned long)arg);
 #if !defined(BROKEN_X86_ALIGNMENT) || defined(CONFIG_X86_X32)
 	/*
-- 
2.20.0