ptrace, seccomp: add support for retrieving seccomp flags

With the new SECCOMP_FILTER_FLAG_LOG, we need to be able to extract these flags for checkpoint restore, since they describe the state of a filter.

So, let's add PTRACE_SECCOMP_GET_FLAGS, similar to ..._GET_FILTER, which returns the flags of the nth filter.

Signed-off-by Tycho Andersen <[email protected]>

CC Kees Cook <[email protected]>

CC Andy Lutomirski <[email protected]>

CC Oleg Nesterov <[email protected]>

---
 include/linux/seccomp.h     |  7 +++++
 include/uapi/linux/ptrace.h |  1 +
 kernel/ptrace.c             |  4 +++
 kernel/seccomp.c            | 74 ++++++++++++++++++++++++++++++++++-----------
 4 files changed, 68 insertions(+), 18 deletions(-)

include/linux/seccomp.h +7 -0

index c8bef436b61d..4713d62378e3 100644
@@ -94,11 +94,18 @@ static inline void get_seccomp_filter(struct task_struct *tsk)
 #if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
 extern long seccomp_get_filter(struct task_struct *task,
                               unsigned long filter_off, void __user *data);
+extern long seccomp_get_flags(struct task_struct *task,
+                             unsigned long filter_off);
 #else
 static inline long seccomp_get_filter(struct task_struct *task,
                                      unsigned long n, void __user *data)
 {
        return -EINVAL;
 }
+static inline long seccomp_get_flags(struct task_struct *task,
+                                    unsigned long filter_off)
+{
+       return -EINVAL;
+}
 #endif /* CONFIG_SECCOMP_FILTER && CONFIG_CHECKPOINT_RESTORE */
 #endif /* _LINUX_SECCOMP_H */

include/uapi/linux/ptrace.h +1 -0

index fb8106509000..52903f0f6600 100644
@@ -65,6 +65,7 @@ struct ptrace_peeksiginfo_args {
 #define PTRACE_SETSIGMASK      0x420b
 
 #define PTRACE_SECCOMP_GET_FILTER      0x420c
+#define PTRACE_SECCOMP_GET_FLAGS       0x420d
 
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED      (1 << 0)

kernel/ptrace.c +4 -0

index 60f356d91060..fa3d2b4d3bf0 100644
@@ -1094,6 +1094,10 @@ int ptrace_request(struct task_struct *child, long request,
                ret = seccomp_get_filter(child, addr, datavp);
                break;
 
+       case PTRACE_SECCOMP_GET_FLAGS:
+               ret = seccomp_get_flags(child, addr);
+               break;
+
        default:
                break;
        }

kernel/seccomp.c +56 -19

index c24579dfa7a1..67768aaa7952 100644
@@ -968,24 +968,16 @@ long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
 }
 
 #if defined(CONFIG_SECCOMP_FILTER) && defined(CONFIG_CHECKPOINT_RESTORE)
-long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
-                       void __user *data)
+static struct seccomp_filter *get_nth_filter(struct task_struct *task,
+                                            unsigned long filter_off)
 {
        struct seccomp_filter *filter;
-       struct sock_fprog_kern *fprog;
-       long ret;
        unsigned long count = 0;
 
-       if (!capable(CAP_SYS_ADMIN) ||
-           current->seccomp.mode != SECCOMP_MODE_DISABLED) {
-               return -EACCES;
-       }
+       WARN_ON_ONCE(!spin_is_locked(&task->sighand->siglock));
 
-       spin_lock_irq(&task->sighand->siglock);
-       if (task->seccomp.mode != SECCOMP_MODE_FILTER) {
-               ret = -EINVAL;
-               goto out;
-       }
+       if (task->seccomp.mode != SECCOMP_MODE_FILTER)
+               return ERR_PTR(-EINVAL);
 
        filter = task->seccomp.filter;
        while (filter) {

@@ -993,10 +985,9 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
                count++;
        }
 
-       if (filter_off >= count) {
-               ret = -ENOENT;
-               goto out;
-       }
+       if (filter_off >= count)
+               return ERR_PTR(-ENOENT);
+
        count -= filter_off;
 
        filter = task->seccomp.filter;

@@ -1007,7 +998,28 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
 
        if (WARN_ON(count != 1 || !filter)) {
                /* The filter tree shouldn't shrink while we're using it. */
-               ret = -ENOENT;
+               return ERR_PTR(-ENOENT);
+       }
+
+       return filter;
+}
+
+long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
+                       void __user *data)
+{
+       struct seccomp_filter *filter;
+       struct sock_fprog_kern *fprog;
+       long ret;
+
+       if (!capable(CAP_SYS_ADMIN) ||
+           current->seccomp.mode != SECCOMP_MODE_DISABLED) {
+               return -EACCES;
+       }
+
+       spin_lock_irq(&task->sighand->siglock);
+       filter = get_nth_filter(task, filter_off);
+       if (IS_ERR(filter)) {
+               ret = PTR_ERR(filter);
                goto out;
        }
 

@@ -1038,6 +1050,32 @@ long seccomp_get_filter(struct task_struct *task, unsigned long filter_off,
        spin_unlock_irq(&task->sighand->siglock);
        return ret;
 }
+
+long seccomp_get_flags(struct task_struct *task, unsigned long filter_off)
+{
+       long flags = 0L;
+       struct seccomp_filter *filter;
+
+       if (!capable(CAP_SYS_ADMIN) ||
+           current->seccomp.mode != SECCOMP_MODE_DISABLED) {
+               return -EACCES;
+       }
+
+       spin_lock_irq(&task->sighand->siglock);
+       filter = get_nth_filter(task, filter_off);
+       if (IS_ERR(filter)) {
+               flags = PTR_ERR(filter);
+               goto out;
+       }
+
+       if (filter->log)
+               flags |= SECCOMP_FILTER_FLAG_LOG;
+
+out:
+       spin_unlock_irq(&task->sighand->siglock);
+       return flags;
+
+}
 #endif
 
 #ifdef CONFIG_SYSCTL
--

2.11.0

Quoted 3 lines · Tycho Andersen

With the new SECCOMP_FILTER_FLAG_LOG, we need to be able to extract these flags for checkpoint restore, since they describe the state of a filter.

Quoted 2 lines

So, let's add PTRACE_SECCOMP_GET_FLAGS, similar to ..._GET_FILTER, which returns the flags of the nth filter.

Can you split this up into factoring out the nth helper, and then adding the new get?

For naming, perhaps "GET_FILTER_FLAGS" instead of "GET_FLAGS" since there may be seccomp flags in the future, etc.

Is there any sane way to add the flags to the existing GET_FILTER?

Hi Kees,

Quoted 4 lines · Kees Cook

Quoted 3 lines · Tycho Andersen

With the new SECCOMP_FILTER_FLAG_LOG, we need to be able to extract these flags for checkpoint restore, since they describe the state of a filter.

Quoted 8 lines

Quoted 2 lines

So, let's add PTRACE_SECCOMP_GET_FLAGS, similar to ..._GET_FILTER, which returns the flags of the nth filter.

Can you split this up into factoring out the nth helper, and then adding the new get?

For naming, perhaps "GET_FILTER_FLAGS" instead of "GET_FLAGS" since there may be seccomp flags in the future, etc.

Sure, I'll do both of these.

Quoted 1 line

Is there any sane way to add the flags to the existing GET_FILTER?

I looked at this, and I don't think so. Unfortunately, we didn't use any structure for the output, it's just the raw bytes of the filter with the length used as the return value. I suppose we could append the flags after the bytes of the filter, but that seems... very ugly :). Let me know if you want to go that route.

Quoted 20 lines · Tycho Andersen

Hi Kees,

Quoted 4 lines · Kees Cook

Quoted 3 lines · Tycho Andersen

With the new SECCOMP_FILTER_FLAG_LOG, we need to be able to extract these flags for checkpoint restore, since they describe the state of a filter.

Quoted 8 lines

Quoted 2 lines

So, let's add PTRACE_SECCOMP_GET_FLAGS, similar to ..._GET_FILTER, which returns the flags of the nth filter.

Can you split this up into factoring out the nth helper, and then adding the new get?

For naming, perhaps "GET_FILTER_FLAGS" instead of "GET_FLAGS" since there may be seccomp flags in the future, etc.

Sure, I'll do both of these.

Quoted 1 line

Is there any sane way to add the flags to the existing GET_FILTER?

Quoted 5 lines

I looked at this, and I don't think so. Unfortunately, we didn't use any structure for the output, it's just the raw bytes of the filter with the length used as the return value. I suppose we could append the flags after the bytes of the filter, but that seems... very ugly :). Let me know if you want to go that route.

I think if we can make the new GET_FILTER_stuff interface more extensible, we should cover any future needs for per-filter content.

Quoted 21 lines · Kees Cook

Quoted 20 lines · Tycho Andersen

Hi Kees,

Quoted 4 lines · Kees Cook

Quoted 3 lines · Tycho Andersen

With the new SECCOMP_FILTER_FLAG_LOG, we need to be able to extract these flags for checkpoint restore, since they describe the state of a filter.

Quoted 8 lines

Quoted 2 lines

So, let's add PTRACE_SECCOMP_GET_FLAGS, similar to ..._GET_FILTER, which returns the flags of the nth filter.

Can you split this up into factoring out the nth helper, and then adding the new get?

For naming, perhaps "GET_FILTER_FLAGS" instead of "GET_FLAGS" since there may be seccomp flags in the future, etc.

Sure, I'll do both of these.

Quoted 1 line

Is there any sane way to add the flags to the existing GET_FILTER?

Quoted 8 lines

Quoted 5 lines

I looked at this, and I don't think so. Unfortunately, we didn't use any structure for the output, it's just the raw bytes of the filter with the length used as the return value. I suppose we could append the flags after the bytes of the filter, but that seems... very ugly :). Let me know if you want to go that route.

I think if we can make the new GET_FILTER_stuff interface more extensible, we should cover any future needs for per-filter content.

Ok, are you interested in something more general than GET_FILTER_FLAGS? Maybe GET_FILTER_METADATA with uapi struct like:

struct seccomp_metadata {
    unsigned int flags;
};

which you call by something like,

struct seccomp_metadata mymeta;

size = ptrace(PTRACE_SECCOMP_GET_FILTER_METADATA, pid, sizeof(mymeta), &mymeta);

if (size < 0) {
    /* error */
}
if (size != sizeof(mymeta)) {
   /* struct seccomp_metadata sizes are different */
}

?