Thread (27 messages) 27 messages, 5 authors, 2014-07-27

Re: [PATCH 10/11] capsicum: prctl(2) to force use of O_BENEATH

From: David Drysdale <hidden>
Date: 2014-07-27 12:08:35
Also in: lkml

On Fri, Jul 25, 2014 at 5:00 PM, Andy Lutomirski [off-list ref] wrote:
On Jul 25, 2014 7:02 AM, "Paolo Bonzini" [off-list ref] wrote:
quoted
Il 25/07/2014 15:47, David Drysdale ha scritto:
quoted
@@ -1996,6 +2013,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
              if (arg2 || arg3 || arg4 || arg5)
                      return -EINVAL;
              return current->no_new_privs ? 1 : 0;
+     case PR_SET_OPENAT_BENEATH:
+             if (arg2 != 1 || arg4 || arg5)
+                     return -EINVAL;
+             if ((arg3 & ~(PR_SET_OPENAT_BENEATH_TSYNC)) != 0)
+                     return -EINVAL;
+             error = prctl_set_openat_beneath(me, arg3);
+             break;
+     case PR_GET_OPENAT_BENEATH:
+             if (arg2 || arg3 || arg4 || arg5)
+                     return -EINVAL;
+             return me->openat_beneath;
      case PR_GET_THP_DISABLE:
              if (arg2 || arg3 || arg4 || arg5)
                      return -EINVAL;
Why are you always forbidding a change of prctl from 1 to 0?  It should
be safe if current->no_new_privs is clear.
I don't immediately see why you're forbidding unsettling it at all.
If you need it to be sticky, then use seccomp or Capsicum to make it
sticky.
Good point, that would make the function more generic -- needing to
latch is specific to Capsicum's use of it.
Also, the way implementation is dangerously racy -- if anyone pokes at
adjacent bitfields without the lock, they can get corrupted.  Try
basing on Kees' seccomp tree or security-next and using the new atomic
flags field.
Ah yes, sorry -- I hadn't yet shifted the implementation to line up with
the work you and Kees have put into the seccomp stuff.

--Andy
quoted
Do new threads inherit from the parent?

Also, I wonder if you need something like this check:

        /*
         * Installing a seccomp filter requires that the task has
         * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
         * This avoids scenarios where unprivileged tasks can affect the
         * behavior of privileged children.
         */
        if (!current->no_new_privs &&
            security_capable_noaudit(current_cred(), current_user_ns(),
                                     CAP_SYS_ADMIN) != 0)
                return -EACCES;

Paolo
Yes, new threads inherit the flag from the parent so the
NNP||CAP_SYS_ADMIN check is probably needed.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help