* [PATCH v3 0/3] binder: use cred instead of task for security context
@ 2021-10-06 19:46 Todd Kjos
2021-10-06 19:46 ` [PATCH v3 1/3] binder: use cred instead of task for selinux checks Todd Kjos
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 19:46 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, Todd Kjos
This series fixes the possible use of an incorrect security context
when checking selinux permissions, getting a security ID, or lookup
up the euid.
The previous behavior was to save the group_leader 'struct task_struct'
in binder_open() and using that to obtain security IDs or euids.
This has been shown to be unreliable, so this series instead saves the
'struct cred' of the task that called binder_open(). This cred is used
for these lookups instead of the task.
v1 and v2 of this series were a single patch "binder: use euid from"
cred instead of using task". During review, Stephen Smalley identified
two more related issues so the corresponding patches were added to
the series.
Todd Kjos (3):
binder: use cred instead of task for selinux checks
binder: use cred instead of task for getsecid
binder: use euid from cred instead of using task
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/3] binder: use cred instead of task for selinux checks
2021-10-06 19:46 [PATCH v3 0/3] binder: use cred instead of task for security context Todd Kjos
@ 2021-10-06 19:46 ` Todd Kjos
2021-10-06 19:46 ` [PATCH v3 2/3] binder: use cred instead of task for getsecid Todd Kjos
2021-10-06 19:46 ` [PATCH v3 3/3] binder: use euid from cred instead of using task Todd Kjos
2 siblings, 0 replies; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 19:46 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, Todd Kjos, stable
Save the struct cred associated with a binder process
at initial open to avoid potential race conditions
when converting to a security ID.
Since binder was integrated with selinux, it has passed
'struct task_struct' associated with the binder_proc
to represent the source and target of transactions.
The conversion of task to SID was then done in the hook
implementations. It turns out that there are race conditions
which can result in an incorrect security context being used.
Fix by saving the 'struct cred' during binder_open and pass
it to the selinux subsystem.
Fixes: 79af73079d75 ("Add security hooks to binder and implement the
hooks for SELinux.")
Suggested-By: Jann Horn <jannh@google.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Todd Kjos <tkjos@google.com>
Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables)
---
v2: updated comments as suggested by Paul Moore
v3: added 2 patches to fix related issues
drivers/android/binder.c | 14 +++++----
drivers/android/binder_internal.h | 4 +++
include/linux/lsm_hook_defs.h | 14 ++++-----
include/linux/lsm_hooks.h | 14 ++++-----
include/linux/security.h | 28 +++++++++---------
security/security.c | 14 ++++-----
security/selinux/hooks.c | 48 +++++++++----------------------
7 files changed, 60 insertions(+), 76 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9edacc8b9768..ca599ebdea4a 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2056,7 +2056,7 @@ static int binder_translate_binder(struct flat_binder_object *fp,
ret = -EINVAL;
goto done;
}
- if (security_binder_transfer_binder(proc->tsk, target_proc->tsk)) {
+ if (security_binder_transfer_binder(proc->cred, target_proc->cred)) {
ret = -EPERM;
goto done;
}
@@ -2102,7 +2102,7 @@ static int binder_translate_handle(struct flat_binder_object *fp,
proc->pid, thread->pid, fp->handle);
return -EINVAL;
}
- if (security_binder_transfer_binder(proc->tsk, target_proc->tsk)) {
+ if (security_binder_transfer_binder(proc->cred, target_proc->cred)) {
ret = -EPERM;
goto done;
}
@@ -2190,7 +2190,7 @@ static int binder_translate_fd(u32 fd, binder_size_t fd_offset,
ret = -EBADF;
goto err_fget;
}
- ret = security_binder_transfer_file(proc->tsk, target_proc->tsk, file);
+ ret = security_binder_transfer_file(proc->cred, target_proc->cred, file);
if (ret < 0) {
ret = -EPERM;
goto err_security;
@@ -2595,8 +2595,8 @@ static void binder_transaction(struct binder_proc *proc,
return_error_line = __LINE__;
goto err_invalid_target_handle;
}
- if (security_binder_transaction(proc->tsk,
- target_proc->tsk) < 0) {
+ if (security_binder_transaction(proc->cred,
+ target_proc->cred) < 0) {
return_error = BR_FAILED_REPLY;
return_error_param = -EPERM;
return_error_line = __LINE__;
@@ -4353,6 +4353,7 @@ static void binder_free_proc(struct binder_proc *proc)
}
binder_alloc_deferred_release(&proc->alloc);
put_task_struct(proc->tsk);
+ put_cred(proc->cred);
binder_stats_deleted(BINDER_STAT_PROC);
kfree(proc);
}
@@ -4564,7 +4565,7 @@ static int binder_ioctl_set_ctx_mgr(struct file *filp,
ret = -EBUSY;
goto out;
}
- ret = security_binder_set_context_mgr(proc->tsk);
+ ret = security_binder_set_context_mgr(proc->cred);
if (ret < 0)
goto out;
if (uid_valid(context->binder_context_mgr_uid)) {
@@ -5055,6 +5056,7 @@ static int binder_open(struct inode *nodp, struct file *filp)
spin_lock_init(&proc->outer_lock);
get_task_struct(current->group_leader);
proc->tsk = current->group_leader;
+ proc->cred = get_cred(filp->f_cred);
INIT_LIST_HEAD(&proc->todo);
init_waitqueue_head(&proc->freeze_wait);
proc->default_priority = task_nice(current);
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 402c4d4362a8..d6b6b8cb7346 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -364,6 +364,9 @@ struct binder_ref {
* (invariant after initialized)
* @tsk task_struct for group_leader of process
* (invariant after initialized)
+ * @cred struct cred associated with the `struct file`
+ * in binder_open()
+ * (invariant after initialized)
* @deferred_work_node: element for binder_deferred_list
* (protected by binder_deferred_lock)
* @deferred_work: bitmap of deferred work to perform
@@ -426,6 +429,7 @@ struct binder_proc {
struct list_head waiting_threads;
int pid;
struct task_struct *tsk;
+ const struct cred *cred;
struct hlist_node deferred_work_node;
int deferred_work;
int outstanding_txns;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 2adeea44c0d5..61590c1f2d33 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -26,13 +26,13 @@
* #undef LSM_HOOK
* };
*/
-LSM_HOOK(int, 0, binder_set_context_mgr, struct task_struct *mgr)
-LSM_HOOK(int, 0, binder_transaction, struct task_struct *from,
- struct task_struct *to)
-LSM_HOOK(int, 0, binder_transfer_binder, struct task_struct *from,
- struct task_struct *to)
-LSM_HOOK(int, 0, binder_transfer_file, struct task_struct *from,
- struct task_struct *to, struct file *file)
+LSM_HOOK(int, 0, binder_set_context_mgr, const struct cred *mgr)
+LSM_HOOK(int, 0, binder_transaction, const struct cred *from,
+ const struct cred *to)
+LSM_HOOK(int, 0, binder_transfer_binder, const struct cred *from,
+ const struct cred *to)
+LSM_HOOK(int, 0, binder_transfer_file, const struct cred *from,
+ const struct cred *to, struct file *file)
LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child,
unsigned int mode)
LSM_HOOK(int, 0, ptrace_traceme, struct task_struct *parent)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5c4c5c0602cb..59024618554e 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1313,22 +1313,22 @@
*
* @binder_set_context_mgr:
* Check whether @mgr is allowed to be the binder context manager.
- * @mgr contains the task_struct for the task being registered.
+ * @mgr contains the struct cred for the current binder process.
* Return 0 if permission is granted.
* @binder_transaction:
* Check whether @from is allowed to invoke a binder transaction call
* to @to.
- * @from contains the task_struct for the sending task.
- * @to contains the task_struct for the receiving task.
+ * @from contains the struct cred for the sending process.
+ * @to contains the struct cred for the receiving process.
* @binder_transfer_binder:
* Check whether @from is allowed to transfer a binder reference to @to.
- * @from contains the task_struct for the sending task.
- * @to contains the task_struct for the receiving task.
+ * @from contains the struct cred for the sending process.
+ * @to contains the struct cred for the receiving process.
* @binder_transfer_file:
* Check whether @from is allowed to transfer @file to @to.
- * @from contains the task_struct for the sending task.
+ * @from contains the struct cred for the sending process.
* @file contains the struct file being transferred.
- * @to contains the task_struct for the receiving task.
+ * @to contains the struct cred for the receiving process.
*
* @ptrace_access_check:
* Check permission before allowing the current process to trace the
diff --git a/include/linux/security.h b/include/linux/security.h
index 5b7288521300..6344d3362df7 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -258,13 +258,13 @@ extern int security_init(void);
extern int early_security_init(void);
/* Security operations */
-int security_binder_set_context_mgr(struct task_struct *mgr);
-int security_binder_transaction(struct task_struct *from,
- struct task_struct *to);
-int security_binder_transfer_binder(struct task_struct *from,
- struct task_struct *to);
-int security_binder_transfer_file(struct task_struct *from,
- struct task_struct *to, struct file *file);
+int security_binder_set_context_mgr(const struct cred *mgr);
+int security_binder_transaction(const struct cred *from,
+ const struct cred *to);
+int security_binder_transfer_binder(const struct cred *from,
+ const struct cred *to);
+int security_binder_transfer_file(const struct cred *from,
+ const struct cred *to, struct file *file);
int security_ptrace_access_check(struct task_struct *child, unsigned int mode);
int security_ptrace_traceme(struct task_struct *parent);
int security_capget(struct task_struct *target,
@@ -508,25 +508,25 @@ static inline int early_security_init(void)
return 0;
}
-static inline int security_binder_set_context_mgr(struct task_struct *mgr)
+static inline int security_binder_set_context_mgr(const struct cred *mgr)
{
return 0;
}
-static inline int security_binder_transaction(struct task_struct *from,
- struct task_struct *to)
+static inline int security_binder_transaction(const struct cred *from,
+ const struct cred *to)
{
return 0;
}
-static inline int security_binder_transfer_binder(struct task_struct *from,
- struct task_struct *to)
+static inline int security_binder_transfer_binder(const struct cred *from,
+ const struct cred *to)
{
return 0;
}
-static inline int security_binder_transfer_file(struct task_struct *from,
- struct task_struct *to,
+static inline int security_binder_transfer_file(const struct cred *from,
+ const struct cred *to,
struct file *file)
{
return 0;
diff --git a/security/security.c b/security/security.c
index 9ffa9e9c5c55..67264cb08fb3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -747,25 +747,25 @@ static int lsm_superblock_alloc(struct super_block *sb)
/* Security operations */
-int security_binder_set_context_mgr(struct task_struct *mgr)
+int security_binder_set_context_mgr(const struct cred *mgr)
{
return call_int_hook(binder_set_context_mgr, 0, mgr);
}
-int security_binder_transaction(struct task_struct *from,
- struct task_struct *to)
+int security_binder_transaction(const struct cred *from,
+ const struct cred *to)
{
return call_int_hook(binder_transaction, 0, from, to);
}
-int security_binder_transfer_binder(struct task_struct *from,
- struct task_struct *to)
+int security_binder_transfer_binder(const struct cred *from,
+ const struct cred *to)
{
return call_int_hook(binder_transfer_binder, 0, from, to);
}
-int security_binder_transfer_file(struct task_struct *from,
- struct task_struct *to, struct file *file)
+int security_binder_transfer_file(const struct cred *from,
+ const struct cred *to, struct file *file)
{
return call_int_hook(binder_transfer_file, 0, from, to, file);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e7ebd45ca345..c8bf3db90c8b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -255,29 +255,6 @@ static inline u32 task_sid_obj(const struct task_struct *task)
return sid;
}
-/*
- * get the security ID of a task for use with binder
- */
-static inline u32 task_sid_binder(const struct task_struct *task)
-{
- /*
- * In many case where this function is used we should be using the
- * task's subjective SID, but we can't reliably access the subjective
- * creds of a task other than our own so we must use the objective
- * creds/SID, 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.
- *
- * If this ever changes and we can safely reference the subjective
- * creds/SID of another task, this function will make it easier to
- * identify the various places where we make use of the task SIDs in
- * the binder code. It is also likely that we will need to adjust
- * the main drivers/android binder code as well.
- */
- return task_sid_obj(task);
-}
-
static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry);
/*
@@ -2066,18 +2043,19 @@ static inline u32 open_file_to_av(struct file *file)
/* Hook functions begin here. */
-static int selinux_binder_set_context_mgr(struct task_struct *mgr)
+static int selinux_binder_set_context_mgr(const struct cred *mgr)
{
return avc_has_perm(&selinux_state,
- current_sid(), task_sid_binder(mgr), SECCLASS_BINDER,
+ current_sid(), cred_sid(mgr), SECCLASS_BINDER,
BINDER__SET_CONTEXT_MGR, NULL);
}
-static int selinux_binder_transaction(struct task_struct *from,
- struct task_struct *to)
+static int selinux_binder_transaction(const struct cred *from,
+ const struct cred *to)
{
u32 mysid = current_sid();
- u32 fromsid = task_sid_binder(from);
+ u32 fromsid = cred_sid(from);
+ u32 tosid = cred_sid(to);
int rc;
if (mysid != fromsid) {
@@ -2088,24 +2066,24 @@ static int selinux_binder_transaction(struct task_struct *from,
return rc;
}
- return avc_has_perm(&selinux_state, fromsid, task_sid_binder(to),
+ return avc_has_perm(&selinux_state, fromsid, tosid,
SECCLASS_BINDER, BINDER__CALL, NULL);
}
-static int selinux_binder_transfer_binder(struct task_struct *from,
- struct task_struct *to)
+static int selinux_binder_transfer_binder(const struct cred *from,
+ const struct cred *to)
{
return avc_has_perm(&selinux_state,
- task_sid_binder(from), task_sid_binder(to),
+ cred_sid(from), cred_sid(to),
SECCLASS_BINDER, BINDER__TRANSFER,
NULL);
}
-static int selinux_binder_transfer_file(struct task_struct *from,
- struct task_struct *to,
+static int selinux_binder_transfer_file(const struct cred *from,
+ const struct cred *to,
struct file *file)
{
- u32 sid = task_sid_binder(to);
+ u32 sid = cred_sid(to);
struct file_security_struct *fsec = selinux_file(file);
struct dentry *dentry = file->f_path.dentry;
struct inode_security_struct *isec;
--
2.33.0.800.g4c38ced690-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] binder: use cred instead of task for getsecid
2021-10-06 19:46 [PATCH v3 0/3] binder: use cred instead of task for security context Todd Kjos
2021-10-06 19:46 ` [PATCH v3 1/3] binder: use cred instead of task for selinux checks Todd Kjos
@ 2021-10-06 19:46 ` Todd Kjos
2021-10-07 0:15 ` kernel test robot
2021-10-06 19:46 ` [PATCH v3 3/3] binder: use euid from cred instead of using task Todd Kjos
2 siblings, 1 reply; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 19:46 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, Todd Kjos, stable
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>
Cc: stable@vger.kernel.org # 5.4+
---
v3: added this patch to series
drivers/android/binder.c | 11 +----------
1 file changed, 1 insertion(+), 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;
--
2.33.0.800.g4c38ced690-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] binder: use euid from cred instead of using task
2021-10-06 19:46 [PATCH v3 0/3] binder: use cred instead of task for security context Todd Kjos
2021-10-06 19:46 ` [PATCH v3 1/3] binder: use cred instead of task for selinux checks Todd Kjos
2021-10-06 19:46 ` [PATCH v3 2/3] binder: use cred instead of task for getsecid Todd Kjos
@ 2021-10-06 19:46 ` Todd Kjos
2021-10-06 19:55 ` Todd Kjos
2 siblings, 1 reply; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 19:46 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, Todd Kjos, stable
Set a transaction's sender_euid from the 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>
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(-)
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_proc *proc,
t->from = thread;
else
t->from = NULL;
- t->sender_euid = task_euid(proc->tsk);
+ t->sender_euid = proc->cred->euid;
t->to_proc = target_proc;
t->to_thread = target_thread;
t->code = tr->code;
--
2.33.0.800.g4c38ced690-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] binder: use euid from cred instead of using task
2021-10-06 19:46 ` [PATCH v3 3/3] binder: use euid from cred instead of using task Todd Kjos
@ 2021-10-06 19:55 ` Todd Kjos
2021-10-06 20:38 ` Todd Kjos
0 siblings, 1 reply; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 19:55 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, stable
On Wed, Oct 6, 2021 at 12:46 PM Todd Kjos <tkjos@google.com> wrote:
>
> Set a transaction's sender_euid from the 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>
> Stephen Smalley <stephen.smalley.work@gmail.com>
This should have been "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(-)
>
> 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_proc *proc,
> t->from = thread;
> else
> t->from = NULL;
> - t->sender_euid = task_euid(proc->tsk);
> + t->sender_euid = proc->cred->euid;
> t->to_proc = target_proc;
> t->to_thread = target_thread;
> t->code = tr->code;
> --
> 2.33.0.800.g4c38ced690-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 3/3] binder: use euid from cred instead of using task
2021-10-06 19:55 ` Todd Kjos
@ 2021-10-06 20:38 ` Todd Kjos
0 siblings, 0 replies; 7+ messages in thread
From: Todd Kjos @ 2021-10-06 20:38 UTC (permalink / raw)
To: gregkh, arve, tkjos, maco, christian, jmorris, serge, paul,
stephen.smalley.work, eparis, keescook, jannh, jeffv, zohar,
linux-security-module, selinux, devel, linux-kernel
Cc: joel, kernel-team, stable
On Wed, Oct 6, 2021 at 12:55 PM Todd Kjos <tkjos@google.com> wrote:
>
> On Wed, Oct 6, 2021 at 12:46 PM Todd Kjos <tkjos@google.com> wrote:
> >
> > Set a transaction's sender_euid from the the 'struct cred'
Sigh...and I forgot to run checkpatch before submitting which would
have caught this "the the"
> > 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>
> > Stephen Smalley <stephen.smalley.work@gmail.com>
>
> This should have been "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(-)
> >
> > 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_proc *proc,
> > t->from = thread;
> > else
> > t->from = NULL;
> > - t->sender_euid = task_euid(proc->tsk);
> > + t->sender_euid = proc->cred->euid;
> > t->to_proc = target_proc;
> > t->to_thread = target_thread;
> > t->code = tr->code;
> > --
> > 2.33.0.800.g4c38ced690-goog
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] binder: use cred instead of task for getsecid
2021-10-06 19:46 ` [PATCH v3 2/3] binder: use cred instead of task for getsecid Todd Kjos
@ 2021-10-07 0:15 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-10-07 0:15 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 31345 bytes --]
Hi Todd,
I love your patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on pcmoore-selinux/next linus/master v5.15-rc3 next-20210922]
[cannot apply to jmorris-security/next-testing]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Todd-Kjos/binder-use-cred-instead-of-task-for-security-context/20211007-034849
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 42ce32b1ae54593e28bc27ec149c04569d3a2e5b
config: s390-randconfig-r044-20211006 (attached as .config)
compiler: s390-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/5b79220d7def1547090a9b6c270d0b60aa9937e5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Todd-Kjos/binder-use-cred-instead-of-task-for-security-context/20211007-034849
git checkout 5b79220d7def1547090a9b6c270d0b60aa9937e5
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=s390
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/android/binder.c: In function 'binder_transaction':
>> drivers/android/binder.c:2725:17: error: implicit declaration of function 'security_cred_getsecid'; did you mean 'security_ipc_getsecid'? [-Werror=implicit-function-declaration]
2725 | security_cred_getsecid(proc->cred, &secid);
| ^~~~~~~~~~~~~~~~~~~~~~
| security_ipc_getsecid
cc1: some warnings being treated as errors
vim +2725 drivers/android/binder.c
2445
2446 static void binder_transaction(struct binder_proc *proc,
2447 struct binder_thread *thread,
2448 struct binder_transaction_data *tr, int reply,
2449 binder_size_t extra_buffers_size)
2450 {
2451 int ret;
2452 struct binder_transaction *t;
2453 struct binder_work *w;
2454 struct binder_work *tcomplete;
2455 binder_size_t buffer_offset = 0;
2456 binder_size_t off_start_offset, off_end_offset;
2457 binder_size_t off_min;
2458 binder_size_t sg_buf_offset, sg_buf_end_offset;
2459 struct binder_proc *target_proc = NULL;
2460 struct binder_thread *target_thread = NULL;
2461 struct binder_node *target_node = NULL;
2462 struct binder_transaction *in_reply_to = NULL;
2463 struct binder_transaction_log_entry *e;
2464 uint32_t return_error = 0;
2465 uint32_t return_error_param = 0;
2466 uint32_t return_error_line = 0;
2467 binder_size_t last_fixup_obj_off = 0;
2468 binder_size_t last_fixup_min_off = 0;
2469 struct binder_context *context = proc->context;
2470 int t_debug_id = atomic_inc_return(&binder_last_id);
2471 char *secctx = NULL;
2472 u32 secctx_sz = 0;
2473
2474 e = binder_transaction_log_add(&binder_transaction_log);
2475 e->debug_id = t_debug_id;
2476 e->call_type = reply ? 2 : !!(tr->flags & TF_ONE_WAY);
2477 e->from_proc = proc->pid;
2478 e->from_thread = thread->pid;
2479 e->target_handle = tr->target.handle;
2480 e->data_size = tr->data_size;
2481 e->offsets_size = tr->offsets_size;
2482 strscpy(e->context_name, proc->context->name, BINDERFS_MAX_NAME);
2483
2484 if (reply) {
2485 binder_inner_proc_lock(proc);
2486 in_reply_to = thread->transaction_stack;
2487 if (in_reply_to == NULL) {
2488 binder_inner_proc_unlock(proc);
2489 binder_user_error("%d:%d got reply transaction with no transaction stack\n",
2490 proc->pid, thread->pid);
2491 return_error = BR_FAILED_REPLY;
2492 return_error_param = -EPROTO;
2493 return_error_line = __LINE__;
2494 goto err_empty_call_stack;
2495 }
2496 if (in_reply_to->to_thread != thread) {
2497 spin_lock(&in_reply_to->lock);
2498 binder_user_error("%d:%d got reply transaction with bad transaction stack, transaction %d has target %d:%d\n",
2499 proc->pid, thread->pid, in_reply_to->debug_id,
2500 in_reply_to->to_proc ?
2501 in_reply_to->to_proc->pid : 0,
2502 in_reply_to->to_thread ?
2503 in_reply_to->to_thread->pid : 0);
2504 spin_unlock(&in_reply_to->lock);
2505 binder_inner_proc_unlock(proc);
2506 return_error = BR_FAILED_REPLY;
2507 return_error_param = -EPROTO;
2508 return_error_line = __LINE__;
2509 in_reply_to = NULL;
2510 goto err_bad_call_stack;
2511 }
2512 thread->transaction_stack = in_reply_to->to_parent;
2513 binder_inner_proc_unlock(proc);
2514 binder_set_nice(in_reply_to->saved_priority);
2515 target_thread = binder_get_txn_from_and_acq_inner(in_reply_to);
2516 if (target_thread == NULL) {
2517 /* annotation for sparse */
2518 __release(&target_thread->proc->inner_lock);
2519 return_error = BR_DEAD_REPLY;
2520 return_error_line = __LINE__;
2521 goto err_dead_binder;
2522 }
2523 if (target_thread->transaction_stack != in_reply_to) {
2524 binder_user_error("%d:%d got reply transaction with bad target transaction stack %d, expected %d\n",
2525 proc->pid, thread->pid,
2526 target_thread->transaction_stack ?
2527 target_thread->transaction_stack->debug_id : 0,
2528 in_reply_to->debug_id);
2529 binder_inner_proc_unlock(target_thread->proc);
2530 return_error = BR_FAILED_REPLY;
2531 return_error_param = -EPROTO;
2532 return_error_line = __LINE__;
2533 in_reply_to = NULL;
2534 target_thread = NULL;
2535 goto err_dead_binder;
2536 }
2537 target_proc = target_thread->proc;
2538 target_proc->tmp_ref++;
2539 binder_inner_proc_unlock(target_thread->proc);
2540 } else {
2541 if (tr->target.handle) {
2542 struct binder_ref *ref;
2543
2544 /*
2545 * There must already be a strong ref
2546 * on this node. If so, do a strong
2547 * increment on the node to ensure it
2548 * stays alive until the transaction is
2549 * done.
2550 */
2551 binder_proc_lock(proc);
2552 ref = binder_get_ref_olocked(proc, tr->target.handle,
2553 true);
2554 if (ref) {
2555 target_node = binder_get_node_refs_for_txn(
2556 ref->node, &target_proc,
2557 &return_error);
2558 } else {
2559 binder_user_error("%d:%d got transaction to invalid handle, %u\n",
2560 proc->pid, thread->pid, tr->target.handle);
2561 return_error = BR_FAILED_REPLY;
2562 }
2563 binder_proc_unlock(proc);
2564 } else {
2565 mutex_lock(&context->context_mgr_node_lock);
2566 target_node = context->binder_context_mgr_node;
2567 if (target_node)
2568 target_node = binder_get_node_refs_for_txn(
2569 target_node, &target_proc,
2570 &return_error);
2571 else
2572 return_error = BR_DEAD_REPLY;
2573 mutex_unlock(&context->context_mgr_node_lock);
2574 if (target_node && target_proc->pid == proc->pid) {
2575 binder_user_error("%d:%d got transaction to context manager from process owning it\n",
2576 proc->pid, thread->pid);
2577 return_error = BR_FAILED_REPLY;
2578 return_error_param = -EINVAL;
2579 return_error_line = __LINE__;
2580 goto err_invalid_target_handle;
2581 }
2582 }
2583 if (!target_node) {
2584 /*
2585 * return_error is set above
2586 */
2587 return_error_param = -EINVAL;
2588 return_error_line = __LINE__;
2589 goto err_dead_binder;
2590 }
2591 e->to_node = target_node->debug_id;
2592 if (WARN_ON(proc == target_proc)) {
2593 return_error = BR_FAILED_REPLY;
2594 return_error_param = -EINVAL;
2595 return_error_line = __LINE__;
2596 goto err_invalid_target_handle;
2597 }
2598 if (security_binder_transaction(proc->cred,
2599 target_proc->cred) < 0) {
2600 return_error = BR_FAILED_REPLY;
2601 return_error_param = -EPERM;
2602 return_error_line = __LINE__;
2603 goto err_invalid_target_handle;
2604 }
2605 binder_inner_proc_lock(proc);
2606
2607 w = list_first_entry_or_null(&thread->todo,
2608 struct binder_work, entry);
2609 if (!(tr->flags & TF_ONE_WAY) && w &&
2610 w->type == BINDER_WORK_TRANSACTION) {
2611 /*
2612 * Do not allow new outgoing transaction from a
2613 * thread that has a transaction at the head of
2614 * its todo list. Only need to check the head
2615 * because binder_select_thread_ilocked picks a
2616 * thread from proc->waiting_threads to enqueue
2617 * the transaction, and nothing is queued to the
2618 * todo list while the thread is on waiting_threads.
2619 */
2620 binder_user_error("%d:%d new transaction not allowed when there is a transaction on thread todo\n",
2621 proc->pid, thread->pid);
2622 binder_inner_proc_unlock(proc);
2623 return_error = BR_FAILED_REPLY;
2624 return_error_param = -EPROTO;
2625 return_error_line = __LINE__;
2626 goto err_bad_todo_list;
2627 }
2628
2629 if (!(tr->flags & TF_ONE_WAY) && thread->transaction_stack) {
2630 struct binder_transaction *tmp;
2631
2632 tmp = thread->transaction_stack;
2633 if (tmp->to_thread != thread) {
2634 spin_lock(&tmp->lock);
2635 binder_user_error("%d:%d got new transaction with bad transaction stack, transaction %d has target %d:%d\n",
2636 proc->pid, thread->pid, tmp->debug_id,
2637 tmp->to_proc ? tmp->to_proc->pid : 0,
2638 tmp->to_thread ?
2639 tmp->to_thread->pid : 0);
2640 spin_unlock(&tmp->lock);
2641 binder_inner_proc_unlock(proc);
2642 return_error = BR_FAILED_REPLY;
2643 return_error_param = -EPROTO;
2644 return_error_line = __LINE__;
2645 goto err_bad_call_stack;
2646 }
2647 while (tmp) {
2648 struct binder_thread *from;
2649
2650 spin_lock(&tmp->lock);
2651 from = tmp->from;
2652 if (from && from->proc == target_proc) {
2653 atomic_inc(&from->tmp_ref);
2654 target_thread = from;
2655 spin_unlock(&tmp->lock);
2656 break;
2657 }
2658 spin_unlock(&tmp->lock);
2659 tmp = tmp->from_parent;
2660 }
2661 }
2662 binder_inner_proc_unlock(proc);
2663 }
2664 if (target_thread)
2665 e->to_thread = target_thread->pid;
2666 e->to_proc = target_proc->pid;
2667
2668 /* TODO: reuse incoming transaction for reply */
2669 t = kzalloc(sizeof(*t), GFP_KERNEL);
2670 if (t == NULL) {
2671 return_error = BR_FAILED_REPLY;
2672 return_error_param = -ENOMEM;
2673 return_error_line = __LINE__;
2674 goto err_alloc_t_failed;
2675 }
2676 INIT_LIST_HEAD(&t->fd_fixups);
2677 binder_stats_created(BINDER_STAT_TRANSACTION);
2678 spin_lock_init(&t->lock);
2679
2680 tcomplete = kzalloc(sizeof(*tcomplete), GFP_KERNEL);
2681 if (tcomplete == NULL) {
2682 return_error = BR_FAILED_REPLY;
2683 return_error_param = -ENOMEM;
2684 return_error_line = __LINE__;
2685 goto err_alloc_tcomplete_failed;
2686 }
2687 binder_stats_created(BINDER_STAT_TRANSACTION_COMPLETE);
2688
2689 t->debug_id = t_debug_id;
2690
2691 if (reply)
2692 binder_debug(BINDER_DEBUG_TRANSACTION,
2693 "%d:%d BC_REPLY %d -> %d:%d, data %016llx-%016llx size %lld-%lld-%lld\n",
2694 proc->pid, thread->pid, t->debug_id,
2695 target_proc->pid, target_thread->pid,
2696 (u64)tr->data.ptr.buffer,
2697 (u64)tr->data.ptr.offsets,
2698 (u64)tr->data_size, (u64)tr->offsets_size,
2699 (u64)extra_buffers_size);
2700 else
2701 binder_debug(BINDER_DEBUG_TRANSACTION,
2702 "%d:%d BC_TRANSACTION %d -> %d - node %d, data %016llx-%016llx size %lld-%lld-%lld\n",
2703 proc->pid, thread->pid, t->debug_id,
2704 target_proc->pid, target_node->debug_id,
2705 (u64)tr->data.ptr.buffer,
2706 (u64)tr->data.ptr.offsets,
2707 (u64)tr->data_size, (u64)tr->offsets_size,
2708 (u64)extra_buffers_size);
2709
2710 if (!reply && !(tr->flags & TF_ONE_WAY))
2711 t->from = thread;
2712 else
2713 t->from = NULL;
2714 t->sender_euid = task_euid(proc->tsk);
2715 t->to_proc = target_proc;
2716 t->to_thread = target_thread;
2717 t->code = tr->code;
2718 t->flags = tr->flags;
2719 t->priority = task_nice(current);
2720
2721 if (target_node && target_node->txn_security_ctx) {
2722 u32 secid;
2723 size_t added_size;
2724
> 2725 security_cred_getsecid(proc->cred, &secid);
2726 ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
2727 if (ret) {
2728 return_error = BR_FAILED_REPLY;
2729 return_error_param = ret;
2730 return_error_line = __LINE__;
2731 goto err_get_secctx_failed;
2732 }
2733 added_size = ALIGN(secctx_sz, sizeof(u64));
2734 extra_buffers_size += added_size;
2735 if (extra_buffers_size < added_size) {
2736 /* integer overflow of extra_buffers_size */
2737 return_error = BR_FAILED_REPLY;
2738 return_error_param = -EINVAL;
2739 return_error_line = __LINE__;
2740 goto err_bad_extra_size;
2741 }
2742 }
2743
2744 trace_binder_transaction(reply, t, target_node);
2745
2746 t->buffer = binder_alloc_new_buf(&target_proc->alloc, tr->data_size,
2747 tr->offsets_size, extra_buffers_size,
2748 !reply && (t->flags & TF_ONE_WAY), current->tgid);
2749 if (IS_ERR(t->buffer)) {
2750 /*
2751 * -ESRCH indicates VMA cleared. The target is dying.
2752 */
2753 return_error_param = PTR_ERR(t->buffer);
2754 return_error = return_error_param == -ESRCH ?
2755 BR_DEAD_REPLY : BR_FAILED_REPLY;
2756 return_error_line = __LINE__;
2757 t->buffer = NULL;
2758 goto err_binder_alloc_buf_failed;
2759 }
2760 if (secctx) {
2761 int err;
2762 size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
2763 ALIGN(tr->offsets_size, sizeof(void *)) +
2764 ALIGN(extra_buffers_size, sizeof(void *)) -
2765 ALIGN(secctx_sz, sizeof(u64));
2766
2767 t->security_ctx = (uintptr_t)t->buffer->user_data + buf_offset;
2768 err = binder_alloc_copy_to_buffer(&target_proc->alloc,
2769 t->buffer, buf_offset,
2770 secctx, secctx_sz);
2771 if (err) {
2772 t->security_ctx = 0;
2773 WARN_ON(1);
2774 }
2775 security_release_secctx(secctx, secctx_sz);
2776 secctx = NULL;
2777 }
2778 t->buffer->debug_id = t->debug_id;
2779 t->buffer->transaction = t;
2780 t->buffer->target_node = target_node;
2781 t->buffer->clear_on_free = !!(t->flags & TF_CLEAR_BUF);
2782 trace_binder_transaction_alloc_buf(t->buffer);
2783
2784 if (binder_alloc_copy_user_to_buffer(
2785 &target_proc->alloc,
2786 t->buffer, 0,
2787 (const void __user *)
2788 (uintptr_t)tr->data.ptr.buffer,
2789 tr->data_size)) {
2790 binder_user_error("%d:%d got transaction with invalid data ptr\n",
2791 proc->pid, thread->pid);
2792 return_error = BR_FAILED_REPLY;
2793 return_error_param = -EFAULT;
2794 return_error_line = __LINE__;
2795 goto err_copy_data_failed;
2796 }
2797 if (binder_alloc_copy_user_to_buffer(
2798 &target_proc->alloc,
2799 t->buffer,
2800 ALIGN(tr->data_size, sizeof(void *)),
2801 (const void __user *)
2802 (uintptr_t)tr->data.ptr.offsets,
2803 tr->offsets_size)) {
2804 binder_user_error("%d:%d got transaction with invalid offsets ptr\n",
2805 proc->pid, thread->pid);
2806 return_error = BR_FAILED_REPLY;
2807 return_error_param = -EFAULT;
2808 return_error_line = __LINE__;
2809 goto err_copy_data_failed;
2810 }
2811 if (!IS_ALIGNED(tr->offsets_size, sizeof(binder_size_t))) {
2812 binder_user_error("%d:%d got transaction with invalid offsets size, %lld\n",
2813 proc->pid, thread->pid, (u64)tr->offsets_size);
2814 return_error = BR_FAILED_REPLY;
2815 return_error_param = -EINVAL;
2816 return_error_line = __LINE__;
2817 goto err_bad_offset;
2818 }
2819 if (!IS_ALIGNED(extra_buffers_size, sizeof(u64))) {
2820 binder_user_error("%d:%d got transaction with unaligned buffers size, %lld\n",
2821 proc->pid, thread->pid,
2822 (u64)extra_buffers_size);
2823 return_error = BR_FAILED_REPLY;
2824 return_error_param = -EINVAL;
2825 return_error_line = __LINE__;
2826 goto err_bad_offset;
2827 }
2828 off_start_offset = ALIGN(tr->data_size, sizeof(void *));
2829 buffer_offset = off_start_offset;
2830 off_end_offset = off_start_offset + tr->offsets_size;
2831 sg_buf_offset = ALIGN(off_end_offset, sizeof(void *));
2832 sg_buf_end_offset = sg_buf_offset + extra_buffers_size -
2833 ALIGN(secctx_sz, sizeof(u64));
2834 off_min = 0;
2835 for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
2836 buffer_offset += sizeof(binder_size_t)) {
2837 struct binder_object_header *hdr;
2838 size_t object_size;
2839 struct binder_object object;
2840 binder_size_t object_offset;
2841
2842 if (binder_alloc_copy_from_buffer(&target_proc->alloc,
2843 &object_offset,
2844 t->buffer,
2845 buffer_offset,
2846 sizeof(object_offset))) {
2847 return_error = BR_FAILED_REPLY;
2848 return_error_param = -EINVAL;
2849 return_error_line = __LINE__;
2850 goto err_bad_offset;
2851 }
2852 object_size = binder_get_object(target_proc, t->buffer,
2853 object_offset, &object);
2854 if (object_size == 0 || object_offset < off_min) {
2855 binder_user_error("%d:%d got transaction with invalid offset (%lld, min %lld max %lld) or object.\n",
2856 proc->pid, thread->pid,
2857 (u64)object_offset,
2858 (u64)off_min,
2859 (u64)t->buffer->data_size);
2860 return_error = BR_FAILED_REPLY;
2861 return_error_param = -EINVAL;
2862 return_error_line = __LINE__;
2863 goto err_bad_offset;
2864 }
2865
2866 hdr = &object.hdr;
2867 off_min = object_offset + object_size;
2868 switch (hdr->type) {
2869 case BINDER_TYPE_BINDER:
2870 case BINDER_TYPE_WEAK_BINDER: {
2871 struct flat_binder_object *fp;
2872
2873 fp = to_flat_binder_object(hdr);
2874 ret = binder_translate_binder(fp, t, thread);
2875
2876 if (ret < 0 ||
2877 binder_alloc_copy_to_buffer(&target_proc->alloc,
2878 t->buffer,
2879 object_offset,
2880 fp, sizeof(*fp))) {
2881 return_error = BR_FAILED_REPLY;
2882 return_error_param = ret;
2883 return_error_line = __LINE__;
2884 goto err_translate_failed;
2885 }
2886 } break;
2887 case BINDER_TYPE_HANDLE:
2888 case BINDER_TYPE_WEAK_HANDLE: {
2889 struct flat_binder_object *fp;
2890
2891 fp = to_flat_binder_object(hdr);
2892 ret = binder_translate_handle(fp, t, thread);
2893 if (ret < 0 ||
2894 binder_alloc_copy_to_buffer(&target_proc->alloc,
2895 t->buffer,
2896 object_offset,
2897 fp, sizeof(*fp))) {
2898 return_error = BR_FAILED_REPLY;
2899 return_error_param = ret;
2900 return_error_line = __LINE__;
2901 goto err_translate_failed;
2902 }
2903 } break;
2904
2905 case BINDER_TYPE_FD: {
2906 struct binder_fd_object *fp = to_binder_fd_object(hdr);
2907 binder_size_t fd_offset = object_offset +
2908 (uintptr_t)&fp->fd - (uintptr_t)fp;
2909 int ret = binder_translate_fd(fp->fd, fd_offset, t,
2910 thread, in_reply_to);
2911
2912 fp->pad_binder = 0;
2913 if (ret < 0 ||
2914 binder_alloc_copy_to_buffer(&target_proc->alloc,
2915 t->buffer,
2916 object_offset,
2917 fp, sizeof(*fp))) {
2918 return_error = BR_FAILED_REPLY;
2919 return_error_param = ret;
2920 return_error_line = __LINE__;
2921 goto err_translate_failed;
2922 }
2923 } break;
2924 case BINDER_TYPE_FDA: {
2925 struct binder_object ptr_object;
2926 binder_size_t parent_offset;
2927 struct binder_fd_array_object *fda =
2928 to_binder_fd_array_object(hdr);
2929 size_t num_valid = (buffer_offset - off_start_offset) /
2930 sizeof(binder_size_t);
2931 struct binder_buffer_object *parent =
2932 binder_validate_ptr(target_proc, t->buffer,
2933 &ptr_object, fda->parent,
2934 off_start_offset,
2935 &parent_offset,
2936 num_valid);
2937 if (!parent) {
2938 binder_user_error("%d:%d got transaction with invalid parent offset or type\n",
2939 proc->pid, thread->pid);
2940 return_error = BR_FAILED_REPLY;
2941 return_error_param = -EINVAL;
2942 return_error_line = __LINE__;
2943 goto err_bad_parent;
2944 }
2945 if (!binder_validate_fixup(target_proc, t->buffer,
2946 off_start_offset,
2947 parent_offset,
2948 fda->parent_offset,
2949 last_fixup_obj_off,
2950 last_fixup_min_off)) {
2951 binder_user_error("%d:%d got transaction with out-of-order buffer fixup\n",
2952 proc->pid, thread->pid);
2953 return_error = BR_FAILED_REPLY;
2954 return_error_param = -EINVAL;
2955 return_error_line = __LINE__;
2956 goto err_bad_parent;
2957 }
2958 ret = binder_translate_fd_array(fda, parent, t, thread,
2959 in_reply_to);
2960 if (ret < 0) {
2961 return_error = BR_FAILED_REPLY;
2962 return_error_param = ret;
2963 return_error_line = __LINE__;
2964 goto err_translate_failed;
2965 }
2966 last_fixup_obj_off = parent_offset;
2967 last_fixup_min_off =
2968 fda->parent_offset + sizeof(u32) * fda->num_fds;
2969 } break;
2970 case BINDER_TYPE_PTR: {
2971 struct binder_buffer_object *bp =
2972 to_binder_buffer_object(hdr);
2973 size_t buf_left = sg_buf_end_offset - sg_buf_offset;
2974 size_t num_valid;
2975
2976 if (bp->length > buf_left) {
2977 binder_user_error("%d:%d got transaction with too large buffer\n",
2978 proc->pid, thread->pid);
2979 return_error = BR_FAILED_REPLY;
2980 return_error_param = -EINVAL;
2981 return_error_line = __LINE__;
2982 goto err_bad_offset;
2983 }
2984 if (binder_alloc_copy_user_to_buffer(
2985 &target_proc->alloc,
2986 t->buffer,
2987 sg_buf_offset,
2988 (const void __user *)
2989 (uintptr_t)bp->buffer,
2990 bp->length)) {
2991 binder_user_error("%d:%d got transaction with invalid offsets ptr\n",
2992 proc->pid, thread->pid);
2993 return_error_param = -EFAULT;
2994 return_error = BR_FAILED_REPLY;
2995 return_error_line = __LINE__;
2996 goto err_copy_data_failed;
2997 }
2998 /* Fixup buffer pointer to target proc address space */
2999 bp->buffer = (uintptr_t)
3000 t->buffer->user_data + sg_buf_offset;
3001 sg_buf_offset += ALIGN(bp->length, sizeof(u64));
3002
3003 num_valid = (buffer_offset - off_start_offset) /
3004 sizeof(binder_size_t);
3005 ret = binder_fixup_parent(t, thread, bp,
3006 off_start_offset,
3007 num_valid,
3008 last_fixup_obj_off,
3009 last_fixup_min_off);
3010 if (ret < 0 ||
3011 binder_alloc_copy_to_buffer(&target_proc->alloc,
3012 t->buffer,
3013 object_offset,
3014 bp, sizeof(*bp))) {
3015 return_error = BR_FAILED_REPLY;
3016 return_error_param = ret;
3017 return_error_line = __LINE__;
3018 goto err_translate_failed;
3019 }
3020 last_fixup_obj_off = object_offset;
3021 last_fixup_min_off = 0;
3022 } break;
3023 default:
3024 binder_user_error("%d:%d got transaction with invalid object type, %x\n",
3025 proc->pid, thread->pid, hdr->type);
3026 return_error = BR_FAILED_REPLY;
3027 return_error_param = -EINVAL;
3028 return_error_line = __LINE__;
3029 goto err_bad_object_type;
3030 }
3031 }
3032 if (t->buffer->oneway_spam_suspect)
3033 tcomplete->type = BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT;
3034 else
3035 tcomplete->type = BINDER_WORK_TRANSACTION_COMPLETE;
3036 t->work.type = BINDER_WORK_TRANSACTION;
3037
3038 if (reply) {
3039 binder_enqueue_thread_work(thread, tcomplete);
3040 binder_inner_proc_lock(target_proc);
3041 if (target_thread->is_dead) {
3042 return_error = BR_DEAD_REPLY;
3043 binder_inner_proc_unlock(target_proc);
3044 goto err_dead_proc_or_thread;
3045 }
3046 BUG_ON(t->buffer->async_transaction != 0);
3047 binder_pop_transaction_ilocked(target_thread, in_reply_to);
3048 binder_enqueue_thread_work_ilocked(target_thread, &t->work);
3049 target_proc->outstanding_txns++;
3050 binder_inner_proc_unlock(target_proc);
3051 wake_up_interruptible_sync(&target_thread->wait);
3052 binder_free_transaction(in_reply_to);
3053 } else if (!(t->flags & TF_ONE_WAY)) {
3054 BUG_ON(t->buffer->async_transaction != 0);
3055 binder_inner_proc_lock(proc);
3056 /*
3057 * Defer the TRANSACTION_COMPLETE, so we don't return to
3058 * userspace immediately; this allows the target process to
3059 * immediately start processing this transaction, reducing
3060 * latency. We will then return the TRANSACTION_COMPLETE when
3061 * the target replies (or there is an error).
3062 */
3063 binder_enqueue_deferred_thread_work_ilocked(thread, tcomplete);
3064 t->need_reply = 1;
3065 t->from_parent = thread->transaction_stack;
3066 thread->transaction_stack = t;
3067 binder_inner_proc_unlock(proc);
3068 return_error = binder_proc_transaction(t,
3069 target_proc, target_thread);
3070 if (return_error) {
3071 binder_inner_proc_lock(proc);
3072 binder_pop_transaction_ilocked(thread, t);
3073 binder_inner_proc_unlock(proc);
3074 goto err_dead_proc_or_thread;
3075 }
3076 } else {
3077 BUG_ON(target_node == NULL);
3078 BUG_ON(t->buffer->async_transaction != 1);
3079 binder_enqueue_thread_work(thread, tcomplete);
3080 return_error = binder_proc_transaction(t, target_proc, NULL);
3081 if (return_error)
3082 goto err_dead_proc_or_thread;
3083 }
3084 if (target_thread)
3085 binder_thread_dec_tmpref(target_thread);
3086 binder_proc_dec_tmpref(target_proc);
3087 if (target_node)
3088 binder_dec_node_tmpref(target_node);
3089 /*
3090 * write barrier to synchronize with initialization
3091 * of log entry
3092 */
3093 smp_wmb();
3094 WRITE_ONCE(e->debug_id_done, t_debug_id);
3095 return;
3096
3097 err_dead_proc_or_thread:
3098 return_error_line = __LINE__;
3099 binder_dequeue_work(proc, tcomplete);
3100 err_translate_failed:
3101 err_bad_object_type:
3102 err_bad_offset:
3103 err_bad_parent:
3104 err_copy_data_failed:
3105 binder_free_txn_fixups(t);
3106 trace_binder_transaction_failed_buffer_release(t->buffer);
3107 binder_transaction_buffer_release(target_proc, NULL, t->buffer,
3108 buffer_offset, true);
3109 if (target_node)
3110 binder_dec_node_tmpref(target_node);
3111 target_node = NULL;
3112 t->buffer->transaction = NULL;
3113 binder_alloc_free_buf(&target_proc->alloc, t->buffer);
3114 err_binder_alloc_buf_failed:
3115 err_bad_extra_size:
3116 if (secctx)
3117 security_release_secctx(secctx, secctx_sz);
3118 err_get_secctx_failed:
3119 kfree(tcomplete);
3120 binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
3121 err_alloc_tcomplete_failed:
3122 if (trace_binder_txn_latency_free_enabled())
3123 binder_txn_latency_free(t);
3124 kfree(t);
3125 binder_stats_deleted(BINDER_STAT_TRANSACTION);
3126 err_alloc_t_failed:
3127 err_bad_todo_list:
3128 err_bad_call_stack:
3129 err_empty_call_stack:
3130 err_dead_binder:
3131 err_invalid_target_handle:
3132 if (target_thread)
3133 binder_thread_dec_tmpref(target_thread);
3134 if (target_proc)
3135 binder_proc_dec_tmpref(target_proc);
3136 if (target_node) {
3137 binder_dec_node(target_node, 1, 0);
3138 binder_dec_node_tmpref(target_node);
3139 }
3140
3141 binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
3142 "%d:%d transaction failed %d/%d, size %lld-%lld line %d\n",
3143 proc->pid, thread->pid, return_error, return_error_param,
3144 (u64)tr->data_size, (u64)tr->offsets_size,
3145 return_error_line);
3146
3147 {
3148 struct binder_transaction_log_entry *fe;
3149
3150 e->return_error = return_error;
3151 e->return_error_param = return_error_param;
3152 e->return_error_line = return_error_line;
3153 fe = binder_transaction_log_add(&binder_transaction_log_failed);
3154 *fe = *e;
3155 /*
3156 * write barrier to synchronize with initialization
3157 * of log entry
3158 */
3159 smp_wmb();
3160 WRITE_ONCE(e->debug_id_done, t_debug_id);
3161 WRITE_ONCE(fe->debug_id_done, t_debug_id);
3162 }
3163
3164 BUG_ON(thread->return_error.cmd != BR_OK);
3165 if (in_reply_to) {
3166 thread->return_error.cmd = BR_TRANSACTION_COMPLETE;
3167 binder_enqueue_thread_work(thread, &thread->return_error.work);
3168 binder_send_failed_reply(in_reply_to, return_error);
3169 } else {
3170 thread->return_error.cmd = return_error;
3171 binder_enqueue_thread_work(thread, &thread->return_error.work);
3172 }
3173 }
3174
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28649 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-07 0:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-06 19:46 [PATCH v3 0/3] binder: use cred instead of task for security context Todd Kjos
2021-10-06 19:46 ` [PATCH v3 1/3] binder: use cred instead of task for selinux checks Todd Kjos
2021-10-06 19:46 ` [PATCH v3 2/3] binder: use cred instead of task for getsecid Todd Kjos
2021-10-07 0:15 ` kernel test robot
2021-10-06 19:46 ` [PATCH v3 3/3] binder: use euid from cred instead of using task Todd Kjos
2021-10-06 19:55 ` Todd Kjos
2021-10-06 20:38 ` Todd Kjos
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.