linux-kernel - Re: [PATCH v4 03/15] fs/fuse: add an idmap argument to fuse_simple

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: <20240904-baugrube-erhoben-b3c1c49a2645@brauner>
Date: Wed, 4 Sep 2024 17:25:40 +0200
From: Christian Brauner <brauner@...nel.org>
To: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
Cc: mszeredi@...hat.com, stgraber@...raber.org, 
	linux-fsdevel@...r.kernel.org, Seth Forshee <sforshee@...nel.org>, 
	Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>, 
	Bernd Schubert <bschubert@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 03/15] fs/fuse: add an idmap argument to
 fuse_simple_request

On Tue, Sep 03, 2024 at 05:16:14PM GMT, Alexander Mikhalitsyn wrote:
> If idmap == NULL *and* filesystem daemon declared idmapped mounts
> support, then uid/gid values in a fuse header will be -1.
> 
> No functional changes intended.
> 
> Cc: Christian Brauner <brauner@...nel.org>
> Cc: Seth Forshee <sforshee@...nel.org>
> Cc: Miklos Szeredi <miklos@...redi.hu>
> Cc: Amir Goldstein <amir73il@...il.com>
> Cc: Bernd Schubert <bschubert@....com>
> Cc: <linux-fsdevel@...r.kernel.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
> ---

Huha, you end up extending fuse_simple_request() with an idmap argument
and passing NULL 38 times and non-NULL only 4 times at the end of this
patch series. That's not pretty. Also, I really dislike passing NULL as
an argument to the idmap helpers. All of the idmapping code uses
nop_mnt_idmap for this case and I think we should the same just with
invalid_mnt_idmap constant.

So I would propose two changes:

(1) Add an extern invalid_mnt_idmap into mnt_idmapping.h and
    define it in fs/mnt_idmapping.c so that will always yield
    INVALID_VFSUID/INVALID_VFSGID. Basically, it's the same definition
    as for nop_mnt_idmap.

(2) Instead of extending fuse_simple_request() with an additional
    argument rename fuse_simple_request() to __fuse_simple_request()
    and extend __fuse_simple_request() with a struct mnt_idmap argument.

    * make fuse_simple_request() a static inline helper that calls
      __fuse_simple_request() with invalid_mnt_idmap in the fuse_i.h
      header.

    * add fuse_idmap_request() that also calls __fuse_simple_request()
      but just passes through the idmap argument.