SELinux: hold tasklist_lock and siglock while waking wait_chldexit

From: Eric Paris <[email protected]>

SELinux has long been calling wake_up_interruptible() on current->parent->signal->wait_chldexit without holding any locks. It appears that this operation should hold the tasklist_lock to dereference current->parent and we should hold the siglock when waking up the signal->wait_chldexit.

---

 security/selinux/hooks.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

index f85597a..1e45c6c 100644
@@ -2269,7 +2269,9 @@ static void selinux_bprm_post_apply_creds(struct linux_binprm *bprm)
 	struct rlimit *rlim, *initrlim;
 	struct itimerval itimer;
 	struct bprm_security_struct *bsec;
+	struct sighand_struct *psig;
 	int rc, i;
+	unsigned long flags;
 
 	tsec = current->security;
 	bsec = bprm->security;

@@ -2330,7 +2332,12 @@ static void selinux_bprm_post_apply_creds(struct linux_binprm *bprm)
 
 	/* Wake up the parent if it is waiting so that it can
 	   recheck wait permission to the new task SID. */
+	read_lock_irq(&tasklist_lock);
+	psig = current->parent->sighand;
+	spin_lock_irqsave(&psig->siglock, flags);
 	wake_up_interruptible(&current->parent->signal->wait_chldexit);
+	spin_unlock_irqrestore(&psig->siglock, flags);
+	read_unlock_irq(&tasklist_lock);
 }
 
 /* superblock security operations */

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

This seems correct to me, too, although I'll just push it to security-testing#next at least for the moment so it gets some testing.

How did you find this issue?

James Morris
<[email protected]>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.

sds over there noticed that we weren't doing any locking during code review and thought it looked off.

I've also poked roland privately to try to get him to take a gander. Who else knows this locking well to tell me if I got it right?

-Eric

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.