From: Alice Ryhl <aliceryhl@google.com>
To: Jann Horn <jannh@google.com>
Cc: "Gary Guo" <gary@garyguo.net>, "Todd Kjos" <tkjos@google.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Arve Hjønnevåg" <arve@android.com>,
"Carlos Llamas" <cmllamas@google.com>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
"Christian Brauner" <brauner@kernel.org>
Subject: Re: [PATCH] rust: task: clarify comments on task UID accessors
Date: Fri, 13 Feb 2026 16:21:38 +0000 [thread overview]
Message-ID: <aY9Pklev_sI30UoD@google.com> (raw)
In-Reply-To: <CAG48ez0=4Tw-FKF5VF+MZNj4sCqi3CTnbB5PHqvC4mo=3s3Rvg@mail.gmail.com>
On Fri, Feb 13, 2026 at 03:43:21PM +0100, Jann Horn wrote:
> On Fri, Feb 13, 2026 at 9:53 AM Gary Guo <gary@garyguo.net> wrote:
> > On Fri Feb 13, 2026 at 2:00 AM CST, Jann Horn wrote:
> > > Linux has separate subjective and objective task credentials, see the
> > > comment above `struct cred`. Clarify which accessor functions operate on
> > > which set of credentials.
> > >
> > > Also document that Task::euid() is a very weird operation. You can see how
> > > weird it is by grepping for task_euid() - binder is its only user.
> > > Task::euid() obtains the objective effective UID - it looks at the
> > > credentials of the task for purposes of acting on it as an object, but then
> > > accesses the effective UID (which the credentials.7 man page describes as
> > > "[...] used by the kernel to determine the permissions that the process
> > > will have when accessing shared resources [...]").
> > >
> > > For context:
> > > Arguably, binder's use of task_euid() is a theoretical security problem,
> > > which only has no impact on Android because Android has no setuid binaries
> > > executable by apps.
> >
> > If there's no setuid binary, then the `task_euid` can also just be replaced with
> > `task_uid`?
>
> That would still be wrong for binder's usecase though.
>
> > > commit 29bc22ac5e5b ("binder: use euid from cred instead of using task")
> > > fixed that by removing that only user of task_euid(), but the fix got
> > > reverted in commit c21a80ca0684 ("binder: fix test regression due to
> > > sender_euid change") because some Android test started failing.
> >
> > What exactly is relying on the current behaviour? It'll better to fix that and
> > remove `task_euid` completely as I find "objective effective UID" quite
> > confusing, even with disclaimer that it shouldn't be used.
>
> I agree with that; I don't know what that failing test was. (Todd
> would probably know.) My understanding is:
>
> In the current version, the ->sender_euid reported to a transaction's
> recipient is the EUID of the sending process *at the time the
> transaction is received by the recipient*. (This is wrong if the
> sending process changed credentials after sending the transaction, and
> especially dangerous if the sending process went through a setuid
> execution in the meantime.)
>
> With the reverted fix, the ->sender_euid reported to the recipient was
> the EUID of the sending process *when it opened /dev/binder*. (That is
> secure, and capturing credentials at open() time is a standard
> mechanism in Linux, but it might be surprising to userspace code that
> calls seteuid() between opening /dev/binder and sending a transaction.
> I guess that's probably what that failing test was about.)
>
> Probably the right fix would be to capture the current_euid() in the
> sending process when it sends a transaction.
We can certainly do that, but I don't know what the test failure was
either. And it should probably be changed for both drivers if so. I just
mirrored what C did.
Alice
next prev parent reply other threads:[~2026-02-13 16:21 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-12 18:00 [PATCH] rust: task: clarify comments on task UID accessors Jann Horn
2026-02-13 8:20 ` Alice Ryhl
2026-02-13 13:44 ` Jann Horn
2026-02-13 8:52 ` Gary Guo
2026-02-13 14:43 ` Jann Horn
2026-02-13 16:21 ` Alice Ryhl [this message]
2026-02-13 21:12 ` Jann Horn
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=aY9Pklev_sI30UoD@google.com \
--to=aliceryhl@google.com \
--cc=a.hindborg@kernel.org \
--cc=arve@android.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=brauner@kernel.org \
--cc=cmllamas@google.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=jannh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tkjos@google.com \
--cc=tmgross@umich.edu \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.