On 01/18, Christian Brauner wrote:
--- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -264,12 +264,17 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state) return ret; } -static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode) +static bool ptrace_has_cap(const struct cred *cred, struct user_namespace *ns,
unsigned int mode){
- int ret;
- if (mode & PTRACE_MODE_NOAUDIT)
return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
ret = security_capable(cred, ns, CAP_SYS_PTRACE, CAP_OPT_NOAUDIT);
return has_ns_capability(current, ns, CAP_SYS_PTRACE);
ret = security_capable(cred, ns, CAP_SYS_PTRACE, CAP_OPT_NONE);- return ret == 0;
} /* Returns 0 on success, -errno on denial. */ @@ -321,7 +326,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) gid_eq(caller_gid, tcred->sgid) && gid_eq(caller_gid, tcred->gid)) goto ok;
- if (ptrace_has_cap(tcred->user_ns, mode))
- if (ptrace_has_cap(cred, tcred->user_ns, mode)) goto ok; rcu_read_unlock(); return -EPERM;
@@ -340,7 +345,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) mm = task->mm; if (mm && ((get_dumpable(mm) != SUID_DUMP_USER) &&
!ptrace_has_cap(mm->user_ns, mode)))
!ptrace_has_cap(cred, mm->user_ns, mode))) return -EPERM;
I never understood these security checks and thus I don't understand the security impact. Say, has_capability_noaudit() in __set_oom_adj(). Isn't it equally wrong?
However, the patch looks "obviously correct" to me.
Reviewed-by: Oleg Nesterov oleg@redhat.com