As Jann pointed out, there is a race between SECCOMP_FILTER_FLAG_TSYNC and the ptrace code that can inspect a filter of another process. Let's introduce read locking into the two ptrace accesses so that we don't race.
Signed-off-by: Tycho Andersen <[email protected]> Reported-by: Jann Horn <[email protected]> CC: Kees Cook <[email protected]> CC: Andy Lutomirski <[email protected]> --- include/linux/seccomp.h | 4 ++-- kernel/seccomp.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h index 8429bdda947a..30b27e898162 100644 --- a/include/linux/seccomp.h +++ b/include/linux/seccomp.h @@ -22,8 +22,8 @@ struct seccomp_filter; * @filter: must always point to a valid seccomp-filter or NULL as it is * accessed without locking during system call entry. * - * @filter must only be accessed from the context of current as there - * is no read locking. + * @filter is read-protected by task->signal->cred_guard_mutex when + * outside of current context. */ struct seccomp { int mode; diff --git a/kernel/seccomp.c b/kernel/seccomp.c index ef80dd19f268..f65d47650ac1 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -1042,7 +1042,12 @@ int seccomp_get_filter(struct task_struct *task, unsigned long filter_off, return -EACCES; } + ret = mutex_lock_killable(&task->signal->cred_guard_mutex); + if (ret < 0) + return ret; + filter = get_nth_filter(task, filter_off); + mutex_unlock(&task->signal->cred_guard_mutex); if (IS_ERR(filter)) return PTR_ERR(filter); @@ -1088,7 +1093,12 @@ int seccomp_get_metadata(struct task_struct *task, if (copy_from_user(&kmd.filter_off, data, sizeof(kmd.filter_off))) return -EFAULT; + ret = mutex_lock_killable(&task->signal->cred_guard_mutex); + if (ret < 0) + return ret; + filter = get_nth_filter(task, kmd.filter_off); + mutex_unlock(&task->signal->cred_guard_mutex); if (IS_ERR(filter)) return PTR_ERR(filter); -- 2.17.1
