Commit 8cac8990 authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'for-linus-2020-01-18' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux

Pull thread fixes from Christian Brauner:
 "Here is an urgent fix for ptrace_may_access() permission checking.

  Commit 69f594a3 ("ptrace: do not audit capability check when
  outputing /proc/pid/stat") introduced the ability to opt out of audit
  messages for accesses to various proc files since they are not
  violations of policy.

  While doing so it switched the check from ns_capable() to
  has_ns_capability{_noaudit}(). That means it switched from checking
  the subjective credentials (ktask->cred) of the task to using the
  objective credentials (ktask->real_cred). This is appears to be wrong.
  ptrace_has_cap() is currently only used in ptrace_may_access() And is
  used to check whether the calling task (subject) has the
  CAP_SYS_PTRACE capability in the provided user namespace to operate on
  the target task (object). According to the cred.h comments this means
  the subjective credentials of the calling task need to be used.

  With this fix we switch ptrace_has_cap() to use security_capable() and
  thus back to using the subjective credentials.

  As one example where this might be particularly problematic, Jann
  pointed out that in combination with the upcoming IORING_OP_OPENAT{2}
  feature, this bug might allow unprivileged users to bypass the
  capability checks while asynchronously opening files like /proc/*/mem,
  because the capability checks for this would be performed against
  kernel credentials.

  To illustrate on the former point about this being exploitable: When
  io_uring creates a new context it records the subjective credentials
  of the caller. Later on, when it starts to do work it creates a kernel
  thread and registers a callback. The callback runs with kernel creds
  for ktask->real_cred and ktask->cred.

  To prevent this from becoming a full-blown 0-day io_uring will call
  override_cred() and override ktask->cred with the subjective
  credentials of the creator of the io_uring instance. With
  ptrace_has_cap() currently looking at ktask->real_cred this override
  will be ineffective and the caller will be able to open arbitray proc
  files as mentioned above.

  Luckily, this is currently not exploitable but would be so once
  IORING_OP_OPENAT{2} land in v5.6. Let's fix it now.

  To minimize potential regressions I successfully ran the criu
  testsuite. criu makes heavy use of ptrace() and extensively hits
  ptrace_may_access() codepaths and has a good change of detecting any
  regressions.

  Additionally, I succesfully ran the ptrace and seccomp kernel tests"

* tag 'for-linus-2020-01-18' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux:
  ptrace: reintroduce usage of subjective credentials in ptrace_has_cap()
parents 2324de6f 6b3ad664
...@@ -264,12 +264,17 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state) ...@@ -264,12 +264,17 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state)
return ret; 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) 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);
else else
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. */ /* Returns 0 on success, -errno on denial. */
...@@ -321,7 +326,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) ...@@ -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->sgid) &&
gid_eq(caller_gid, tcred->gid)) gid_eq(caller_gid, tcred->gid))
goto ok; goto ok;
if (ptrace_has_cap(tcred->user_ns, mode)) if (ptrace_has_cap(cred, tcred->user_ns, mode))
goto ok; goto ok;
rcu_read_unlock(); rcu_read_unlock();
return -EPERM; return -EPERM;
...@@ -340,7 +345,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) ...@@ -340,7 +345,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
mm = task->mm; mm = task->mm;
if (mm && if (mm &&
((get_dumpable(mm) != SUID_DUMP_USER) && ((get_dumpable(mm) != SUID_DUMP_USER) &&
!ptrace_has_cap(mm->user_ns, mode))) !ptrace_has_cap(cred, mm->user_ns, mode)))
return -EPERM; return -EPERM;
return security_ptrace_access_check(task, mode); return security_ptrace_access_check(task, mode);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment