From: Jann Horn <jannh@google.com>
To: Miklos Szeredi <miklos@szeredi.hu>,
Eric Biederman <ebiederm@xmission.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] fuse: Remove user_ns check for FUSE_DEV_IOC_CLONE
Date: Wed, 14 Sep 2022 16:26:32 +0200 [thread overview]
Message-ID: <20220914142632.2016571-1-jannh@google.com> (raw)
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>
---
@Eric: Does this look reasonable to you? I dug through my old mails,
and in the off-list discussion back then, Linus and you were in favor
of adding this check.
fs/fuse/dev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 51897427a5346..920480054e1dc 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2266,8 +2266,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
* Check against file->f_op because CUSE
* uses the same ioctl handler.
*/
- if (old->f_op == file->f_op &&
- old->f_cred->user_ns == file->f_cred->user_ns)
+ if (old->f_op == file->f_op)
fud = fuse_get_dev(old);
if (fud) {
base-commit: 3245cb65fd91cd514801bf91f5a3066d562f0ac4
--
2.37.2.789.g6183377224-goog
next reply other threads:[~2022-09-14 14:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-14 14:26 Jann Horn [this message]
2022-10-12 13:46 ` [PATCH] fuse: Remove user_ns check for FUSE_DEV_IOC_CLONE Miklos Szeredi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220914142632.2016571-1-jannh@google.com \
--to=jannh@google.com \
--cc=ebiederm@xmission.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).