Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220914142632.2016571-1-jannh@google.com>
In-Reply-To: <20220914142632.2016571-1-jannh@google.com>
From:   Miklos Szeredi <miklos@szeredi.hu>
Date:   Wed, 12 Oct 2022 15:46:06 +0200
Message-ID: <CAJfpegsncvnDtTHEv=qT0dGoD4B4L=cukyFwwVtaq8MKiWFQjw@mail.gmail.com>
Subject: Re: [PATCH] fuse: Remove user_ns check for FUSE_DEV_IOC_CLONE
To:     Jann Horn <jannh@google.com>
Cc:     Eric Biederman <ebiederm@xmission.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 14 Sept 2022 at 16:27, Jann Horn <jannh@google.com> wrote:
>
> Commit 8ed1f0e22f49e ("fs/fuse: fix ioctl type confusion") fixed a type
> confusion bug by adding an ->f_op comparison.
>
> Based on some off-list discussion back then, another check was added to
> compare the f_cred->user_ns. This is not for security reasons, but was
> based on the idea that a FUSE device FD should be using the UID/GID
> mappings of its f_cred->user_ns, and those translations are done using
> fc->user_ns, which matches the f_cred->user_ns of the initial
> FUSE device FD thanks to the check in fuse_fill_super().
> See also commit 8cb08329b0809 ("fuse: Support fuse filesystems outside of
> init_user_ns").
>
> But FUSE_DEV_IOC_CLONE is, at a higher level, a *cloning* operation that
> copies an existing context (with a weird API that involves first opening
> /dev/fuse, then tying the resulting new FUSE device FD to an existing FUSE
> instance). So if an application is already passing FUSE FDs across
> userns boundaries and dealing with the resulting ID mapping complications
> somehow, it doesn't make much sense to block this cloning operation.
>
> I've heard that this check is an obstacle for some folks, and I don't see
> a good reason to keep it, so remove it.
>
> Signed-off-by: Jann Horn <jannh@google.com>

I see no issues with this.   f_cred seems to be unused by the VFS, so
this should have zero effect on anything other than rejecting or
allowing FUSE_DEV_IOC_CLONE.

Applied.

Thanks,
Miklos