linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Stas Sergeev <stsp2@yandex.ru>
Cc: linux-kernel@vger.kernel.org,
	"Stefan Metzmacher" <metze@samba.org>,
	"Eric Biederman" <ebiederm@xmission.com>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Brauner" <brauner@kernel.org>,
	"Jan Kara" <jack@suse.cz>, "Jeff Layton" <jlayton@kernel.org>,
	"Chuck Lever" <chuck.lever@oracle.com>,
	"Alexander Aring" <alex.aring@gmail.com>,
	"David Laight" <David.Laight@aculab.com>,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Christian Göttsche" <cgzones@googlemail.com>
Subject: Re: [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag
Date: Thu, 25 Apr 2024 03:31:27 +0100	[thread overview]
Message-ID: <20240425023127.GH2118490@ZenIV> (raw)
In-Reply-To: <20240424105248.189032-3-stsp2@yandex.ru>

On Wed, Apr 24, 2024 at 01:52:48PM +0300, Stas Sergeev wrote:
> @@ -3793,8 +3828,23 @@ static struct file *path_openat(struct nameidata *nd,
>  			error = do_o_path(nd, flags, file);
>  	} else {
>  		const char *s = path_init(nd, flags);
> -		file = alloc_empty_file(open_flags, current_cred());
> -		error = PTR_ERR_OR_ZERO(file);
> +		const struct cred *old_cred = NULL;
> +
> +		error = 0;
> +		if (open_flags & OA2_INHERIT_CRED) {
> +			/* Only work with O_CLOEXEC dirs. */
> +			if (!get_close_on_exec(nd->dfd))
> +				error = -EPERM;
> +
> +			if (!error)
> +				old_cred = openat2_override_creds(nd);
> +		}
> +		if (!error) {
> +			file = alloc_empty_file(open_flags, current_cred());

Consider the following, currently absolutely harmless situation:
	* process is owned by luser:students.
	* descriptor 69 refers to root-opened root directory (O_RDONLY)
What's the expected result of
	fcntl(69, F_SEFTD, O_CLOEXEC);
	opening "etc/shadow" with dirfd equal to 69 and your flag given
	subsequent read() from the resulting descriptor?

At which point will the kernel say "go fuck yourself, I'm not letting you
read that file", provided that attacker passes that new flag of yours?

As a bonus question, how about opening it for _write_, seeing that this
is an obvious instant roothole?

Again, currently the setup that has a root-opened directory in descriptor
table of a non-root process is safe.

Incidentally, suppose you have the same process run with stdin opened
(r/o) by root.  F_SETFD it to O_CLOEXEC, then use your open with
dirfd being 0, pathname - "" and flags - O_RDWR.

AFAICS, without an explicit opt-in by the original opener it's
a non-starter, and TBH I doubt that even with such opt-in (FMODE_CRED,
whatever) it would be a good idea - it gives too much.

NAKed-by: Al Viro <viro@zeniv.linux.org.uk>

  reply	other threads:[~2024-04-25  2:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 10:52 [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2() Stas Sergeev
2024-04-24 10:52 ` [PATCH 1/2] fs: reorganize path_openat() Stas Sergeev
2024-04-25  8:13   ` kernel test robot
2024-04-24 10:52 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
2024-04-25  2:31   ` Al Viro [this message]
2024-04-25  7:24     ` stsp
2024-04-25  9:23     ` stsp
2024-04-25 13:50   ` kernel test robot
2024-04-25 14:02   ` Christian Brauner
2024-04-26 13:36     ` stsp
2024-04-24 16:09 ` [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2() Christian Brauner
2024-04-24 17:50   ` stsp
2024-04-25  9:54     ` Christian Brauner
2024-04-25 10:12       ` stsp
2024-04-25 12:08         ` Christian Brauner
2024-04-25 12:39           ` stsp
  -- strict thread matches above, loose matches on Subject: below --
2024-04-23 22:46 [PATCH v3 " Stas Sergeev
2024-04-23 22:46 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
2024-04-23 11:01 [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2() Stas Sergeev
2024-04-23 11:01 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
2024-04-23 10:48 [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2() Stas Sergeev
2024-04-23 10:48 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
     [not found] <20240423104002.9012-1-stsp2@yandex.ru>
2024-04-23 10:40 ` Stas Sergeev
2024-04-22  8:45 [PATCH 1/2] fs: reorganize path_openat() Stas Sergeev
2024-04-22  8:45 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
2024-04-22 19:53   ` Stefan Metzmacher
2024-04-22 20:18     ` stsp
2024-04-23 22:59     ` stsp

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=20240425023127.GH2118490@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=David.Laight@aculab.com \
    --cc=alex.aring@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cgzones@googlemail.com \
    --cc=chuck.lever@oracle.com \
    --cc=ebiederm@xmission.com \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=metze@samba.org \
    --cc=pbonzini@redhat.com \
    --cc=stsp2@yandex.ru \
    /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).