linux-ext4 - Re: [PATCH v4] fcntl: Add 32bit filesystem mode

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: <b63c04ff68340d367ad4138f3496d217df9b5151.camel@icenowy.me>
Date:   Thu, 19 May 2022 22:23:08 +0800
From:   Icenowy Zheng <uwu@...nowy.me>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>
Cc:     linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-api@...r.kernel.org, qemu-devel@...gnu.org,
        Florian Weimer <fw@...eb.enyo.de>,
        Peter Maydell <peter.maydell@...aro.org>,
        Andy Lutomirski <luto@...nel.org>,
        Eric Blake <eblake@...hat.com>,
        罗勇刚 <luoyonggang@...il.com>
Subject: Re: [PATCH v4] fcntl: Add 32bit filesystem mode

在 2020-11-18星期三的 00:39 +0100，Linus Walleij写道：
> It was brought to my attention that this bug from 2018 was
> still unresolved: 32 bit emulators like QEMU were given
> 64 bit hashes when running 32 bit emulation on 64 bit systems.
> 

Sorry for replying such an old mail, but I found that using 32-bit file
syscalls in 32-bit QEMU user on 64-bit hosts are still broken today,
and google sent me here.

This mail does not get any reply according to linux-ext4 patchwork, so
could I ping it?

Thanks,
Icenowy Zheng

> This adds a flag to the fcntl() F_GETFD and F_SETFD operations
> to set the underlying filesystem into 32bit mode even if the
> file handle was opened using 64bit mode without the compat
> syscalls.
> 
> Programs that need the 32 bit file system behavior need to
> issue a fcntl() system call such as in this example:
> 
>   #define FD_32BIT_MODE 2
> 
>   int main(int argc, char** argv) {
>     DIR* dir;
>     int err;
>     int mode;
>     int fd;
> 
>     dir = opendir("/boot");
>     fd = dirfd(dir);
>     mode = fcntl(fd, F_GETFD);
>     mode |= FD_32BIT_MODE;
>     err = fcntl(fd, F_SETFD, mode);
>     if (err) {
>       printf("fcntl() failed! err=%d\n", err);
>       return 1;
>     }
>     printf("dir=%p\n", dir);
>     printf("readdir(dir)=%p\n", readdir(dir));
>     printf("errno=%d: %s\n", errno, strerror(errno));
>     return 0;
>   }
> 
> This can be pretty hard to test since C libraries and linux
> userspace security extensions aggressively filter the parameters
> that are passed down and allowed to commit into actual system
> calls.
> 
> Cc: Florian Weimer <fw@...eb.enyo.de>
> Cc: Peter Maydell <peter.maydell@...aro.org>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: Eric Blake <eblake@...hat.com>
> Reported-by: 罗勇刚(Yonggang Luo) <luoyonggang@...il.com>
> Suggested-by: Theodore Ts'o <tytso@....edu>
> Link: https://bugs.launchpad.net/qemu/+bug/1805913
> Link: https://lore.kernel.org/lkml/87bm56vqg4.fsf@mid.deneb.enyo.de/
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=205957
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
> ChangeLog v3 RESEND 1-> v4:
> - Update the example in the commit message to a read/modify/write
>   version.
> - Notice that Yonggang Luo sees the sema problem on i386 binaries
>   as we see on ARM 32bit binaries.
> ChangeLog v3->v3 RESEND 1:
> - Resending during the v5.10 merge window to get attention.
> ChangeLog v2->v3:
> - Realized that I also have to clear the flag correspondingly
>   if someone ask for !FD_32BIT_MODE after setting it the
>   first time.
> ChangeLog v1->v2:
> - Use a new flag FD_32BIT_MODE to F_GETFD and F_SETFD
>   instead of a new fcntl operation, there is already a fcntl
>   operation to set random flags.
> - Sorry for taking forever to respin this patch :(
> ---
>  fs/fcntl.c                       | 7 +++++++
>  include/uapi/asm-generic/fcntl.h | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 19ac5baad50f..6c32edc4099a 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -335,10 +335,17 @@ static long do_fcntl(int fd, unsigned int cmd,
> unsigned long arg,
>                 break;
>         case F_GETFD:
>                 err = get_close_on_exec(fd) ? FD_CLOEXEC : 0;
> +               /* Report 32bit file system mode */
> +               if (filp->f_mode & FMODE_32BITHASH)
> +                       err |= FD_32BIT_MODE;
>                 break;
>         case F_SETFD:
>                 err = 0;
>                 set_close_on_exec(fd, arg & FD_CLOEXEC);
> +               if (arg & FD_32BIT_MODE)
> +                       filp->f_mode |= FMODE_32BITHASH;
> +               else
> +                       filp->f_mode &= ~FMODE_32BITHASH;
>                 break;
>         case F_GETFL:
>                 err = filp->f_flags;
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-
> generic/fcntl.h
> index 9dc0bf0c5a6e..edd3573cb7ef 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -160,6 +160,14 @@ struct f_owner_ex {
>  
>  /* for F_[GET|SET]FL */
>  #define FD_CLOEXEC     1       /* actually anything with low bit set
> goes */
> +/*
> + * This instructs the kernel to provide 32bit semantics (such as
> hashes) from
> + * the file system layer, when running a userland that depend on 32bit
> + * semantics on a kernel that supports 64bit userland, but does not
> use the
> + * compat ioctl() for e.g. open(), so that the kernel would otherwise
> assume
> + * that the userland process is capable of dealing with 64bit
> semantics.
> + */
> +#define FD_32BIT_MODE  2
>  
>  /* for posix fcntl() and lockf() */
>  #ifndef F_RDLCK