Re: [PATCH v3 1/3] ptrace: Provide ___ptrace_may_access() that can be applied on arbitrary tasks

Tim Chen
2018-09-04T17:26:50Z

On 09/04/2018 07:40 AM, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
>
> Current ptrace_may_access() implementation assumes that the 'source' task is
> always the caller (current).
>
> Expose ___ptrace_may_access() that can be used to apply the check on arbitrary
> tasks.

Casey recently has proposed putting the decision making of whether to
do IBPB in the security module.

https://lwn.net/ml/kernel-hardening/20180815235355.14908-4-casey.schaufler@intel.com/

That will have the advantage of giving the administrator a more flexibility
of when to turn on IBPB. The policy is very similar to what you have proposed here
but I think the security module is a more appropriate place for the security policy.

Thanks.

Tim

>
> Originally-by: Tim Chen <tim.c.chen@linux.intel.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
>
> Sorry for the resend, my pine is buggered and broke threading.
>
> include/linux/ptrace.h | 12 ++++++++++++
> kernel/ptrace.c | 13 ++++++++++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 4f36431c380b..09744d4113fb 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -87,6 +87,18 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
> */
> extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
>
> +/**
> + * ___ptrace_may_access - variant of ptrace_may_access that can be used
> + * between two arbitrary tasks
> + * @curr: source task
> + * @task: target task
> + * @mode: selects type of access and caller credentials
> + *
> + * Returns true on success, false on denial.
> + */
> +extern int ___ptrace_may_access(struct task_struct *curr, struct task_struct *task,
> + unsigned int mode);
> +
> static inline int ptrace_reparented(struct task_struct *child)
> {
> return !same_thread_group(child->real_parent, child->parent);
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 21fec73d45d4..07ff6685ebed 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -268,9 +268,10 @@ static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
> }
>
> /* Returns 0 on success, -errno on denial. */
> -static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +int ___ptrace_may_access(struct task_struct *curr, struct task_struct *task,
> + unsigned int mode)
> {
> - const struct cred *cred = current_cred(), *tcred;
> + const struct cred *cred, *tcred;
> struct mm_struct *mm;
> kuid_t caller_uid;
> kgid_t caller_gid;
> @@ -290,9 +291,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> */
>
> /* Don't let security modules deny introspection */
> - if (same_thread_group(task, current))
> + if (same_thread_group(task, curr))
> return 0;
> rcu_read_lock();
> + cred = __task_cred(curr);
> if (mode & PTRACE_MODE_FSCREDS) {
> caller_uid = cred->fsuid;
> caller_gid = cred->fsgid;
> @@ -331,6 +333,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> return security_ptrace_access_check(task, mode);
> }
>
> +static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +{
> + return ___ptrace_may_access(current, task, mode);
> +}
> +
> bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> {
> int err;
>