From a811dc61559e0c8003f1086c2a4dc8e4d5ae4cb8 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Sat, 12 Jan 2019 11:24:20 -0700 Subject: [PATCH 1/3] seccomp: fix UAF in user-trap code On the failure path, we do an fput() of the listener fd if the filter fails to install (e.g. because of a TSYNC race that's lost, or if the thread is killed, etc.). fput() doesn't actually release the fd, it just ads it to a work queue. Then the thread proceeds to free the filter, even though the listener struct file has a reference to it. To fix this, on the failure path let's set the private data to null, so we know in ->release() to ignore the filter. Reported-by: syzbot+981c26489b2d1c6316ba@syzkaller.appspotmail.com Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") Signed-off-by: Tycho Andersen Acked-by: Kees Cook Signed-off-by: James Morris --- kernel/seccomp.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kernel/seccomp.c b/kernel/seccomp.c index d7f538847b84..e815781ed751 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -976,6 +976,9 @@ static int seccomp_notify_release(struct inode *inode, struct file *file) struct seccomp_filter *filter = file->private_data; struct seccomp_knotif *knotif; + if (!filter) + return 0; + mutex_lock(&filter->notify_lock); /* @@ -1300,6 +1303,7 @@ out: out_put_fd: if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) { if (ret < 0) { + listener_f->private_data = NULL; fput(listener_f); put_unused_fd(listener); } else { From 9474f4e7cd71a633fa1ef93b7daefd44bbdfd482 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 16 Jan 2019 10:31:09 -0800 Subject: [PATCH 2/3] Yama: Check for pid death before checking ancestry It's possible that a pid has died before we take the rcu lock, in which case we can't walk the ancestry list as it may be detached. Instead, check for death first before doing the walk. Reported-by: syzbot+a9ac39bf55329e206219@syzkaller.appspotmail.com Fixes: 2d514487faf1 ("security: Yama LSM") Cc: stable@vger.kernel.org Suggested-by: Oleg Nesterov Signed-off-by: Kees Cook Signed-off-by: James Morris --- security/yama/yama_lsm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index ffda91a4a1aa..02514fe558b4 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -368,7 +368,9 @@ static int yama_ptrace_access_check(struct task_struct *child, break; case YAMA_SCOPE_RELATIONAL: rcu_read_lock(); - if (!task_is_descendant(current, child) && + if (!pid_alive(child)) + rc = -EPERM; + if (!rc && !task_is_descendant(current, child) && !ptracer_exception_found(current, child) && !ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE)) rc = -EPERM; From a5795fd38ee8194451ba3f281f075301a3696ce2 Mon Sep 17 00:00:00 2001 From: James Morris Date: Wed, 16 Jan 2019 15:41:11 -0800 Subject: [PATCH 3/3] LSM: Check for NULL cred-security on free From: Casey Schaufler Check that the cred security blob has been set before trying to clean it up. There is a case during credential initialization that could result in this. Signed-off-by: Casey Schaufler Acked-by: John Johansen Signed-off-by: James Morris Reported-by: syzbot+69ca07954461f189e808@syzkaller.appspotmail.com --- security/security.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/security/security.c b/security/security.c index f1b8d2587639..55bc49027ba9 100644 --- a/security/security.c +++ b/security/security.c @@ -1027,6 +1027,13 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp) void security_cred_free(struct cred *cred) { + /* + * There is a failure case in prepare_creds() that + * may result in a call here with ->security being NULL. + */ + if (unlikely(cred->security == NULL)) + return; + call_void_hook(cred_free, cred); }