All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, yangshukui <yangshukui@huawei.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Casey Schaufler <casey@schaufler-ca.com>,
	Oleg Nesterov <oleg@redhat.com>, Paul Moore <paul@paul-moore.com>,
	Alexander Grund <theflamefire89@gmail.com>
Subject: [PATCH 4.9 01/26] security,selinux,smack: kill security_task_wait hook
Date: Wed, 27 Jul 2022 18:10:30 +0200	[thread overview]
Message-ID: <20220727160959.177769342@linuxfoundation.org> (raw)
In-Reply-To: <20220727160959.122591422@linuxfoundation.org>

From: Stephen Smalley <sds@tycho.nsa.gov>

commit 3a2f5a59a695a73e0cde9a61e0feae5fa730e936 upstream.

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: yangshukui <yangshukui@huawei.com>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Alexander Grund <theflamefire89@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/lsm_hooks.h  |    7 -------
 include/linux/security.h   |    6 ------
 kernel/exit.c              |   19 ++-----------------
 security/security.c        |    6 ------
 security/selinux/hooks.c   |    6 ------
 security/smack/smack_lsm.c |   20 --------------------
 6 files changed, 2 insertions(+), 62 deletions(-)

--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -666,11 +666,6 @@
  *	@sig contains the signal value.
  *	@secid contains the sid of the process where the signal originated
  *	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:
  *	Check permission before performing a process control operation on the
  *	current process.
@@ -1507,7 +1502,6 @@ union security_list_options {
 	int (*task_movememory)(struct task_struct *p);
 	int (*task_kill)(struct task_struct *p, struct siginfo *info,
 				int sig, u32 secid);
-	int (*task_wait)(struct task_struct *p);
 	int (*task_prctl)(int option, unsigned long arg2, unsigned long arg3,
 				unsigned long arg4, unsigned long arg5);
 	void (*task_to_inode)(struct task_struct *p, struct inode *inode);
@@ -1768,7 +1762,6 @@ struct security_hook_heads {
 	struct list_head task_getscheduler;
 	struct list_head task_movememory;
 	struct list_head task_kill;
-	struct list_head task_wait;
 	struct list_head task_prctl;
 	struct list_head task_to_inode;
 	struct list_head ipc_permission;
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -332,7 +332,6 @@ int security_task_getscheduler(struct ta
 int security_task_movememory(struct task_struct *p);
 int security_task_kill(struct task_struct *p, struct siginfo *info,
 			int sig, u32 secid);
-int security_task_wait(struct task_struct *p);
 int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 			unsigned long arg4, unsigned long arg5);
 void security_task_to_inode(struct task_struct *p, struct inode *inode);
@@ -979,11 +978,6 @@ static inline int security_task_kill(str
 {
 	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,
 				      unsigned long arg3,
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -14,7 +14,6 @@
 #include <linux/tty.h>
 #include <linux/iocontext.h>
 #include <linux/key.h>
-#include <linux/security.h>
 #include <linux/cpu.h>
 #include <linux/acct.h>
 #include <linux/tsacct_kern.h>
@@ -1342,7 +1341,7 @@ static int wait_task_continued(struct wa
  * Returns nonzero for a final return, when we have unlocked tasklist_lock.
  * Returns zero if the search for a child should continue;
  * 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,
 				struct task_struct *p)
@@ -1362,20 +1361,6 @@ static int wait_consider_task(struct wai
 	if (!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)) {
 		/*
 		 * ptrace == 0 means we are the natural parent. In this case
@@ -1468,7 +1453,7 @@ static int wait_consider_task(struct wai
  * Returns nonzero for a final return, when we have unlocked tasklist_lock.
  * Returns zero if the search for a child should continue; then
  * ->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)
 {
--- a/security/security.c
+++ b/security/security.c
@@ -1032,11 +1032,6 @@ int security_task_kill(struct task_struc
 	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,
 			 unsigned long arg4, unsigned long arg5)
 {
@@ -1776,7 +1771,6 @@ struct security_hook_heads security_hook
 	.task_movememory =
 		LIST_HEAD_INIT(security_hook_heads.task_movememory),
 	.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_to_inode =
 		LIST_HEAD_INIT(security_hook_heads.task_to_inode),
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3951,11 +3951,6 @@ static int selinux_task_kill(struct task
 	return rc;
 }
 
-static int selinux_task_wait(struct task_struct *p)
-{
-	return task_has_perm(p, current, PROCESS__SIGCHLD);
-}
-
 static void selinux_task_to_inode(struct task_struct *p,
 				  struct inode *inode)
 {
@@ -6220,7 +6215,6 @@ static struct security_hook_list selinux
 	LSM_HOOK_INIT(task_getscheduler, selinux_task_getscheduler),
 	LSM_HOOK_INIT(task_movememory, selinux_task_movememory),
 	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(ipc_permission, selinux_ipc_permission),
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2277,25 +2277,6 @@ static int smack_task_kill(struct task_s
 }
 
 /**
- * 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
  * @p: task to copy from
  * @inode: inode to copy to
@@ -4686,7 +4667,6 @@ static struct security_hook_list smack_h
 	LSM_HOOK_INIT(task_getscheduler, smack_task_getscheduler),
 	LSM_HOOK_INIT(task_movememory, smack_task_movememory),
 	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(ipc_permission, smack_ipc_permission),



  reply	other threads:[~2022-07-27 16:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-27 16:10 [PATCH 4.9 00/26] 4.9.325-rc1 review Greg Kroah-Hartman
2022-07-27 16:10 ` Greg Kroah-Hartman [this message]
2022-07-27 16:10 ` [PATCH 4.9 02/26] xen/gntdev: Ignore failure to unmap INVALID_GRANT_HANDLE Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 03/26] misc: rtsx_usb: fix use of dma mapped buffer for usb bulk transfer Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 04/26] misc: rtsx_usb: use separate command and response buffers Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 05/26] misc: rtsx_usb: set return value in rsp_buf alloc err path Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 06/26] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 07/26] power/reset: arm-versatile: Fix refcount leak in versatile_reboot_probe Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 08/26] perf/core: Fix data race between perf_event_set_output() and perf_mmap_close() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 09/26] ip: Fix a data-race around sysctl_fwmark_reflect Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 10/26] tcp/dccp: Fix a data-race around sysctl_tcp_fwmark_accept Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 11/26] tcp: Fix a data-race around sysctl_tcp_probe_threshold Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 12/26] i2c: cadence: Change large transfer count reset logic to be unconditional Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 13/26] igmp: Fix data-races around sysctl_igmp_llm_reports Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 14/26] igmp: Fix a data-race around sysctl_igmp_max_memberships Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 15/26] tcp: Fix a data-race around sysctl_tcp_notsent_lowat Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 16/26] be2net: Fix buffer overflow in be_get_module_eeprom Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 17/26] Revert "Revert "char/random: silence a lockdep splat with printk()"" Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 18/26] mm/mempolicy: fix uninit-value in mpol_rebind_policy() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 19/26] bpf: Make sure mac_header was set before using it Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 20/26] ALSA: memalloc: Align buffer allocations in page size Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 21/26] tty: drivers/tty/, stop using tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 22/26] tty: the rest, " Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 23/26] tty: drop tty_schedule_flip() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 24/26] tty: extract tty_flip_buffer_commit() from tty_flip_buffer_push() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 25/26] tty: use new tty_insert_flip_string_and_push_buffer() in pty_write() Greg Kroah-Hartman
2022-07-27 16:10 ` [PATCH 4.9 26/26] net: usb: ax88179_178a needs FLAG_SEND_ZLP Greg Kroah-Hartman
2022-07-27 22:18 ` [PATCH 4.9 00/26] 4.9.325-rc1 review Florian Fainelli
2022-07-28  9:42 ` Pavel Machek
2022-07-28 10:08 ` Naresh Kamboju
2022-07-28 14:32 ` Jon Hunter
2022-07-28 14:52 ` Shuah Khan
2022-07-28 22:57 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220727160959.177769342@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=casey@schaufler-ca.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=sds@tycho.nsa.gov \
    --cc=stable@vger.kernel.org \
    --cc=theflamefire89@gmail.com \
    --cc=yangshukui@huawei.com \
    --subject='Re: [PATCH 4.9 01/26] security,selinux,smack: kill security_task_wait hook' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.