Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Oleg Nesterov <oleg@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, Wen Yang <wen.yang99@zte.com.cn>,
        majiang <ma.jiang@zte.com.cn>,
        "J. Bruce Fields" <bfields@fieldses.org>,
        syzkaller-bugs@googlegroups.com
References: <87efft5ncd.fsf_-_@xmission.com>
        <20180724032419.20231-7-ebiederm@xmission.com>
Date:   Wed, 15 Aug 2018 23:04:07 -0500
In-Reply-To: <20180724032419.20231-7-ebiederm@xmission.com> (Eric
        W. Biederman's message of "Mon, 23 Jul 2018 22:24:06 -0500")
Message-ID: <87k1orgdoo.fsf_-_@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: [PATCH] signal: Don't send signals to tasks that don't exist
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Recently syzbot reported crashes in send_sigio_to_task and
send_sigurg_to_task in linux-next.  Despite finding a reproducer
syzbot apparently did not bisected this or otherwise track down the
offending commit in linux-next.

I happened to see this report and examined the code because I had
recently changed these functions as part of making PIDTYPE_TGID a real
pid type so that fork would does not need to restart when receiving a
signal.  By examination I see that I spotted a bug in the code
that could explain the reported crashes.

When I took Oleg's suggestion and optimized send_sigurg and send_sigio
to only send to a single task when type is PIDTYPE_PID or PIDTYPE_TGID
I failed to handle pids that no longer point to tasks.  The macro
do_each_pid_task simply iterates for zero iterations.  With pid_task
an explicit NULL test is needed.

Update the code to include the missing NULL test.

Fixes: 019191342fec ("signal: Use PIDTYPE_TGID to clearly store where file signals will be sent")
Reported-by: syzkaller-bugs@googlegroups.com
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/fcntl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index a04accf6847f..4137d96534a6 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -791,7 +791,8 @@ void send_sigio(struct fown_struct *fown, int fd, int band)
 	if (type <= PIDTYPE_TGID) {
 		rcu_read_lock();
 		p = pid_task(pid, PIDTYPE_PID);
-		send_sigio_to_task(p, fown, fd, band, type);
+		if (p)
+			send_sigio_to_task(p, fown, fd, band, type);
 		rcu_read_unlock();
 	} else {
 		read_lock(&tasklist_lock);
@@ -830,7 +831,8 @@ int send_sigurg(struct fown_struct *fown)
 	if (type <= PIDTYPE_TGID) {
 		rcu_read_lock();
 		p = pid_task(pid, PIDTYPE_PID);
-		send_sigurg_to_task(p, fown, type);
+		if (p)
+			send_sigurg_to_task(p, fown, type);
 		rcu_read_unlock();
 	} else {
 		read_lock(&tasklist_lock);
-- 
2.17.1