Commit 3a2f5a59 authored by Stephen Smalley's avatar Stephen Smalley Committed by Paul Moore

security,selinux,smack: kill security_task_wait hook

As reported by yangshukui, a permission denial from security_task_wait()
can lead to a soft lockup in zap_pid_ns_processes() since it only expects
sys_wait4() to return 0 or -ECHILD. Further, security_task_wait() can
in general lead to zombies; in the absence of some way to automatically
reparent a child process upon a denial, the hook is not useful.  Remove
the security hook and its implementations in SELinux and Smack.  Smack
already removed its check from its hook.
Reported-by: default avataryangshukui <yangshukui@huawei.com>
Signed-off-by: default avatarStephen Smalley <sds@tycho.nsa.gov>
Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
Acked-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
parent b4ba35c7
...@@ -666,11 +666,6 @@ ...@@ -666,11 +666,6 @@
* @sig contains the signal value. * @sig contains the signal value.
* @secid contains the sid of the process where the signal originated * @secid contains the sid of the process where the signal originated
* Return 0 if permission is granted. * Return 0 if permission is granted.
* @task_wait:
* Check permission before allowing a process to reap a child process @p
* and collect its status information.
* @p contains the task_struct for process.
* Return 0 if permission is granted.
* @task_prctl: * @task_prctl:
* Check permission before performing a process control operation on the * Check permission before performing a process control operation on the
* current process. * current process.
...@@ -1507,7 +1502,6 @@ union security_list_options { ...@@ -1507,7 +1502,6 @@ union security_list_options {
int (*task_movememory)(struct task_struct *p); int (*task_movememory)(struct task_struct *p);
int (*task_kill)(struct task_struct *p, struct siginfo *info, int (*task_kill)(struct task_struct *p, struct siginfo *info,
int sig, u32 secid); int sig, u32 secid);
int (*task_wait)(struct task_struct *p);
int (*task_prctl)(int option, unsigned long arg2, unsigned long arg3, int (*task_prctl)(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5); unsigned long arg4, unsigned long arg5);
void (*task_to_inode)(struct task_struct *p, struct inode *inode); void (*task_to_inode)(struct task_struct *p, struct inode *inode);
...@@ -1767,7 +1761,6 @@ struct security_hook_heads { ...@@ -1767,7 +1761,6 @@ struct security_hook_heads {
struct list_head task_getscheduler; struct list_head task_getscheduler;
struct list_head task_movememory; struct list_head task_movememory;
struct list_head task_kill; struct list_head task_kill;
struct list_head task_wait;
struct list_head task_prctl; struct list_head task_prctl;
struct list_head task_to_inode; struct list_head task_to_inode;
struct list_head ipc_permission; struct list_head ipc_permission;
......
...@@ -332,7 +332,6 @@ int security_task_getscheduler(struct task_struct *p); ...@@ -332,7 +332,6 @@ int security_task_getscheduler(struct task_struct *p);
int security_task_movememory(struct task_struct *p); int security_task_movememory(struct task_struct *p);
int security_task_kill(struct task_struct *p, struct siginfo *info, int security_task_kill(struct task_struct *p, struct siginfo *info,
int sig, u32 secid); int sig, u32 secid);
int security_task_wait(struct task_struct *p);
int security_task_prctl(int option, unsigned long arg2, unsigned long arg3, int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5); unsigned long arg4, unsigned long arg5);
void security_task_to_inode(struct task_struct *p, struct inode *inode); void security_task_to_inode(struct task_struct *p, struct inode *inode);
...@@ -980,11 +979,6 @@ static inline int security_task_kill(struct task_struct *p, ...@@ -980,11 +979,6 @@ static inline int security_task_kill(struct task_struct *p,
return 0; return 0;
} }
static inline int security_task_wait(struct task_struct *p)
{
return 0;
}
static inline int security_task_prctl(int option, unsigned long arg2, static inline int security_task_prctl(int option, unsigned long arg2,
unsigned long arg3, unsigned long arg3,
unsigned long arg4, unsigned long arg4,
......
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
#include <linux/tty.h> #include <linux/tty.h>
#include <linux/iocontext.h> #include <linux/iocontext.h>
#include <linux/key.h> #include <linux/key.h>
#include <linux/security.h>
#include <linux/cpu.h> #include <linux/cpu.h>
#include <linux/acct.h> #include <linux/acct.h>
#include <linux/tsacct_kern.h> #include <linux/tsacct_kern.h>
...@@ -1360,7 +1359,7 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p) ...@@ -1360,7 +1359,7 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p)
* Returns nonzero for a final return, when we have unlocked tasklist_lock. * Returns nonzero for a final return, when we have unlocked tasklist_lock.
* Returns zero if the search for a child should continue; * Returns zero if the search for a child should continue;
* then ->notask_error is 0 if @p is an eligible child, * then ->notask_error is 0 if @p is an eligible child,
* or another error from security_task_wait(), or still -ECHILD. * or still -ECHILD.
*/ */
static int wait_consider_task(struct wait_opts *wo, int ptrace, static int wait_consider_task(struct wait_opts *wo, int ptrace,
struct task_struct *p) struct task_struct *p)
...@@ -1380,20 +1379,6 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace, ...@@ -1380,20 +1379,6 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
if (!ret) if (!ret)
return ret; return ret;
ret = security_task_wait(p);
if (unlikely(ret < 0)) {
/*
* If we have not yet seen any eligible child,
* then let this error code replace -ECHILD.
* A permission error will give the user a clue
* to look for security policy problems, rather
* than for mysterious wait bugs.
*/
if (wo->notask_error)
wo->notask_error = ret;
return 0;
}
if (unlikely(exit_state == EXIT_TRACE)) { if (unlikely(exit_state == EXIT_TRACE)) {
/* /*
* ptrace == 0 means we are the natural parent. In this case * ptrace == 0 means we are the natural parent. In this case
...@@ -1486,7 +1471,7 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace, ...@@ -1486,7 +1471,7 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
* Returns nonzero for a final return, when we have unlocked tasklist_lock. * Returns nonzero for a final return, when we have unlocked tasklist_lock.
* Returns zero if the search for a child should continue; then * Returns zero if the search for a child should continue; then
* ->notask_error is 0 if there were any eligible children, * ->notask_error is 0 if there were any eligible children,
* or another error from security_task_wait(), or still -ECHILD. * or still -ECHILD.
*/ */
static int do_wait_thread(struct wait_opts *wo, struct task_struct *tsk) static int do_wait_thread(struct wait_opts *wo, struct task_struct *tsk)
{ {
......
...@@ -1025,11 +1025,6 @@ int security_task_kill(struct task_struct *p, struct siginfo *info, ...@@ -1025,11 +1025,6 @@ int security_task_kill(struct task_struct *p, struct siginfo *info,
return call_int_hook(task_kill, 0, p, info, sig, secid); return call_int_hook(task_kill, 0, p, info, sig, secid);
} }
int security_task_wait(struct task_struct *p)
{
return call_int_hook(task_wait, 0, p);
}
int security_task_prctl(int option, unsigned long arg2, unsigned long arg3, int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5) unsigned long arg4, unsigned long arg5)
{ {
...@@ -1769,7 +1764,6 @@ struct security_hook_heads security_hook_heads = { ...@@ -1769,7 +1764,6 @@ struct security_hook_heads security_hook_heads = {
.task_movememory = .task_movememory =
LIST_HEAD_INIT(security_hook_heads.task_movememory), LIST_HEAD_INIT(security_hook_heads.task_movememory),
.task_kill = LIST_HEAD_INIT(security_hook_heads.task_kill), .task_kill = LIST_HEAD_INIT(security_hook_heads.task_kill),
.task_wait = LIST_HEAD_INIT(security_hook_heads.task_wait),
.task_prctl = LIST_HEAD_INIT(security_hook_heads.task_prctl), .task_prctl = LIST_HEAD_INIT(security_hook_heads.task_prctl),
.task_to_inode = .task_to_inode =
LIST_HEAD_INIT(security_hook_heads.task_to_inode), LIST_HEAD_INIT(security_hook_heads.task_to_inode),
......
...@@ -3963,12 +3963,6 @@ static int selinux_task_kill(struct task_struct *p, struct siginfo *info, ...@@ -3963,12 +3963,6 @@ static int selinux_task_kill(struct task_struct *p, struct siginfo *info,
return avc_has_perm(secid, task_sid(p), SECCLASS_PROCESS, perm, NULL); return avc_has_perm(secid, task_sid(p), SECCLASS_PROCESS, perm, NULL);
} }
static int selinux_task_wait(struct task_struct *p)
{
return avc_has_perm(task_sid(p), current_sid(), SECCLASS_PROCESS,
PROCESS__SIGCHLD, NULL);
}
static void selinux_task_to_inode(struct task_struct *p, static void selinux_task_to_inode(struct task_struct *p,
struct inode *inode) struct inode *inode)
{ {
...@@ -6211,7 +6205,6 @@ static struct security_hook_list selinux_hooks[] = { ...@@ -6211,7 +6205,6 @@ static struct security_hook_list selinux_hooks[] = {
LSM_HOOK_INIT(task_getscheduler, selinux_task_getscheduler), LSM_HOOK_INIT(task_getscheduler, selinux_task_getscheduler),
LSM_HOOK_INIT(task_movememory, selinux_task_movememory), LSM_HOOK_INIT(task_movememory, selinux_task_movememory),
LSM_HOOK_INIT(task_kill, selinux_task_kill), LSM_HOOK_INIT(task_kill, selinux_task_kill),
LSM_HOOK_INIT(task_wait, selinux_task_wait),
LSM_HOOK_INIT(task_to_inode, selinux_task_to_inode), LSM_HOOK_INIT(task_to_inode, selinux_task_to_inode),
LSM_HOOK_INIT(ipc_permission, selinux_ipc_permission), LSM_HOOK_INIT(ipc_permission, selinux_ipc_permission),
......
...@@ -2271,25 +2271,6 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info, ...@@ -2271,25 +2271,6 @@ static int smack_task_kill(struct task_struct *p, struct siginfo *info,
return rc; return rc;
} }
/**
* smack_task_wait - Smack access check for waiting
* @p: task to wait for
*
* Returns 0
*/
static int smack_task_wait(struct task_struct *p)
{
/*
* Allow the operation to succeed.
* Zombies are bad.
* In userless environments (e.g. phones) programs
* get marked with SMACK64EXEC and even if the parent
* and child shouldn't be talking the parent still
* may expect to know when the child exits.
*/
return 0;
}
/** /**
* smack_task_to_inode - copy task smack into the inode blob * smack_task_to_inode - copy task smack into the inode blob
* @p: task to copy from * @p: task to copy from
...@@ -4658,7 +4639,6 @@ static struct security_hook_list smack_hooks[] = { ...@@ -4658,7 +4639,6 @@ static struct security_hook_list smack_hooks[] = {
LSM_HOOK_INIT(task_getscheduler, smack_task_getscheduler), LSM_HOOK_INIT(task_getscheduler, smack_task_getscheduler),
LSM_HOOK_INIT(task_movememory, smack_task_movememory), LSM_HOOK_INIT(task_movememory, smack_task_movememory),
LSM_HOOK_INIT(task_kill, smack_task_kill), LSM_HOOK_INIT(task_kill, smack_task_kill),
LSM_HOOK_INIT(task_wait, smack_task_wait),
LSM_HOOK_INIT(task_to_inode, smack_task_to_inode), LSM_HOOK_INIT(task_to_inode, smack_task_to_inode),
LSM_HOOK_INIT(ipc_permission, smack_ipc_permission), LSM_HOOK_INIT(ipc_permission, smack_ipc_permission),
......
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