On Mon, Oct 11, 2021 at 5:59 PM Casey Schaufler casey@schaufler-ca.com wrote:
On 10/11/2021 2:33 PM, Paul Moore wrote:
On Wed, Oct 6, 2021 at 8:46 PM Todd Kjos tkjos@google.com wrote:
Use the 'struct cred' saved at binder_open() to lookup the security ID via security_cred_getsecid(). This ensures that the security context that opened binder is the one used to generate the secctx.
Fixes: ec74136ded79 ("binder: create node flag to request sender's security context") Signed-off-by: Todd Kjos tkjos@google.com Suggested-by: Stephen Smalley stephen.smalley.work@gmail.com Reported-by: kernel test robot lkp@intel.com Cc: stable@vger.kernel.org # 5.4+
v3: added this patch to series v4: fix build-break for !CONFIG_SECURITY
drivers/android/binder.c | 11 +---------- include/linux/security.h | 4 ++++ 2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c index ca599ebdea4a..989afd0804ca 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2722,16 +2722,7 @@ static void binder_transaction(struct binder_proc *proc, u32 secid; size_t added_size;
/** Arguably this should be the task's subjective LSM secid but* we can't reliably access the subjective creds of a task* other than our own so we must use the objective creds, which* are safe to access. The downside is that if a task is* temporarily overriding it's creds it will not be reflected* here; however, it isn't clear that binder would handle that* case well anyway.*/security_task_getsecid_obj(proc->tsk, &secid);
security_cred_getsecid(proc->cred, &secid); ret = security_secid_to_secctx(secid, &secctx, &secctx_sz); if (ret) { return_error = BR_FAILED_REPLY;diff --git a/include/linux/security.h b/include/linux/security.h index 6344d3362df7..f02cc0211b10 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1041,6 +1041,10 @@ static inline void security_transfer_creds(struct cred *new, { }
+static inline void security_cred_getsecid(const struct cred *c, u32 *secid) +{ +}
Since security_cred_getsecid() doesn't return an error code we should probably set the secid to 0 in this case, for example:
static inline void security_cred_getsecid(...) { *secid = 0; }
If CONFIG_SECURITY is unset there shouldn't be any case where the secid value is ever used for anything. Are you suggesting that it be set out of an abundance of caution?
It follows a pattern with the other LSM hooks when !CONFIG_SECURITY, and I'd much rather us keep things consistent.