Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4 3/3] binder: use euid from cred instead of using task
To:     Paul Moore <paul@paul-moore.com>, Todd Kjos <tkjos@google.com>
Cc:     gregkh@linuxfoundation.org, arve@android.com, tkjos@android.com,
        maco@android.com, christian@brauner.io,
        James Morris <jmorris@namei.org>,
        Serge Hallyn <serge@hallyn.com>,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>, keescook@chromium.org,
        jannh@google.com, Jeffrey Vander Stoep <jeffv@google.com>,
        zohar@linux.ibm.com, linux-security-module@vger.kernel.org,
        selinux@vger.kernel.org, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org, joel@joelfernandes.org,
        kernel-team@android.com, stable@vger.kernel.org,
        Casey Schaufler <casey@schaufler-ca.com>
References: <20211007004629.1113572-1-tkjos@google.com>
 <20211007004629.1113572-4-tkjos@google.com>
 <CAHC9VhTRTcZW9eyXXvAN7T=ZCQ_zwH5iBz+d0h2ntf7=XHE-Vw@mail.gmail.com>
From:   Casey Schaufler <casey@schaufler-ca.com>
Message-ID: <6dd3cdff-c4eb-6457-f04c-199263acd80b@schaufler-ca.com>
Date:   Fri, 8 Oct 2021 14:25:39 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <CAHC9VhTRTcZW9eyXXvAN7T=ZCQ_zwH5iBz+d0h2ntf7=XHE-Vw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Content-Language: en-US
Content-Length: 2029
Precedence: bulk

On 10/8/2021 2:12 PM, Paul Moore wrote:
> On Wed, Oct 6, 2021 at 8:46 PM Todd Kjos <tkjos@google.com> wrote:
>> Set a transaction's sender_euid from the 'struct cred'
>> saved at binder_open() instead of looking up the euid
>> from the binder proc's 'struct task'. This ensures
>> the euid is associated with the security context that
>> of the task that opened binder.
>>
>> Fixes: 457b9a6f09f0 ("Staging: android: add binder driver")
>> Signed-off-by: Todd Kjos <tkjos@google.com>
>> Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>> Cc: stable@vger.kernel.org # 4.4+
>> ---
>> v3: added this patch to series
>>
>>  drivers/android/binder.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> This is an interesting ordering of the patches.  Unless I'm missing
> something I would have expected patch 3/3 to come first, followed by
> 2/3, with patch 1/3 at the end; basically the reverse of what was
> posted here.
>
> My reading of the previous thread was that Casey has made his peace
> with these changes

Yes. I will address the stacking concerns more directly.
I am still somewhat baffled by the intent of the hook, the data
passed to it, and the SELinux policy enforcement decisions, but
that's beyond my scope.

>  so unless anyone has any objections I'll plan on
> merging 2/3 and 3/3 into selinux/stable-5.15 and merging 1/3 into
> selinux/next.
>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 989afd0804ca..26382e982c5e 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -2711,7 +2711,7 @@ static void binder_transaction(struct binder_pro=
c *proc,
>>                 t->from =3D thread;
>>         else
>>                 t->from =3D NULL;
>> -       t->sender_euid =3D task_euid(proc->tsk);
>> +       t->sender_euid =3D proc->cred->euid;
>>         t->to_proc =3D target_proc;
>>         t->to_thread =3D target_thread;
>>         t->code =3D tr->code;
>> --
>> 2.33.0.800.g4c38ced690-goog
> --
> paul moore
> www.paul-moore.com