linux-kernel - Re: [PATCH v5 03/18] gfs2: add compat

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAHc6FU5n9rBZuH=chOdmGCwyrX-B-Euq8oFrnu3UHHKSm5A5gQ@mail.gmail.com>
Date:   Fri, 16 Aug 2019 19:31:47 +0200
From:   Andreas Gruenbacher <agruenba@...hat.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Bob Peterson <rpeterso@...hat.com>,
        Steve Whitehouse <swhiteho@...hat.com>,
        Jan Kara <jack@...e.cz>, NeilBrown <neilb@...e.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        cluster-devel <cluster-devel@...hat.com>
Subject: Re: [PATCH v5 03/18] gfs2: add compat_ioctl support

Arnd,

On Wed, Aug 14, 2019 at 10:45 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> Out of the four ioctl commands supported on gfs2, only FITRIM
> works in compat mode.
>
> Add a proper handler based on the ext4 implementation.
>
> Fixes: 6ddc5c3ddf25 ("gfs2: getlabel support")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  fs/gfs2/file.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 52fa1ef8400b..49287f0b96d0 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -6,6 +6,7 @@
>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/compat.h>
>  #include <linux/completion.h>
>  #include <linux/buffer_head.h>
>  #include <linux/pagemap.h>
> @@ -354,6 +355,25 @@ static long gfs2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>         return -ENOTTY;
>  }
>
> +#ifdef CONFIG_COMPAT
> +static long gfs2_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> +       /* These are just misnamed, they actually get/put from/to user an int */
> +       switch(cmd) {
> +       case FS_IOC32_GETFLAGS:
> +               cmd = FS_IOC_GETFLAGS;
> +               break;
> +       case FS_IOC32_SETFLAGS:
> +               cmd = FS_IOC_SETFLAGS;
> +               break;

I'd like the code to be more explicit here:

        case FITRIM:
        case FS_IOC_GETFSLABEL:
              break;
        default:
              return -ENOIOCTLCMD;

> +       }
> +
> +       return gfs2_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#else
> +#define gfs2_compat_ioctl NULL
> +#endif
> +
>  /**
>   * gfs2_size_hint - Give a hint to the size of a write request
>   * @filep: The struct file
> @@ -1294,6 +1314,7 @@ const struct file_operations gfs2_file_fops = {
>         .write_iter     = gfs2_file_write_iter,
>         .iopoll         = iomap_dio_iopoll,
>         .unlocked_ioctl = gfs2_ioctl,
> +       .compat_ioctl   = gfs2_compat_ioctl,
>         .mmap           = gfs2_mmap,
>         .open           = gfs2_open,
>         .release        = gfs2_release,
> @@ -1309,6 +1330,7 @@ const struct file_operations gfs2_file_fops = {
>  const struct file_operations gfs2_dir_fops = {
>         .iterate_shared = gfs2_readdir,
>         .unlocked_ioctl = gfs2_ioctl,
> +       .compat_ioctl   = gfs2_compat_ioctl,
>         .open           = gfs2_open,
>         .release        = gfs2_release,
>         .fsync          = gfs2_fsync,
> @@ -1325,6 +1347,7 @@ const struct file_operations gfs2_file_fops_nolock = {
>         .write_iter     = gfs2_file_write_iter,
>         .iopoll         = iomap_dio_iopoll,
>         .unlocked_ioctl = gfs2_ioctl,
> +       .compat_ioctl   = gfs2_compat_ioctl,
>         .mmap           = gfs2_mmap,
>         .open           = gfs2_open,
>         .release        = gfs2_release,
> @@ -1338,6 +1361,7 @@ const struct file_operations gfs2_file_fops_nolock = {
>  const struct file_operations gfs2_dir_fops_nolock = {
>         .iterate_shared = gfs2_readdir,
>         .unlocked_ioctl = gfs2_ioctl,
> +       .compat_ioctl   = gfs2_compat_ioctl,
>         .open           = gfs2_open,
>         .release        = gfs2_release,
>         .fsync          = gfs2_fsync,
> --
> 2.20.0
>

Should we feed this through the gfs2 tree?

Thanks,
Andreas