linux-kernel - [PATCH 3.16 029/305] signal/GenWQE: Fix sending of SIGKILL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1549201508.186807772@decadent.org.uk>
Date:   Sun, 03 Feb 2019 14:45:08 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Kleber Sacilotto de Souza" <klebers@...ux.vnet.ibm.com>,
        "Michael Ruettger" <michael@...ra.de>,
        "Joerg-Stephan Vogt" <jsvogt@...ibm.com>,
        "Eberhard S. Amann" <esa@...ux.vnet.ibm.com>,
        "Frank Haverkamp" <haver@...ux.vnet.ibm.com>,
        "Gabriel Krisman Bertazi" <krisman@...ux.vnet.ibm.com>,
        "Michael Jung" <mijung@....net>,
        "Guilherme G. Piccoli" <gpiccoli@...ux.vnet.ibm.com>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        "Sebastian Ott" <sebott@...ux.vnet.ibm.com>
Subject: [PATCH 3.16 029/305] signal/GenWQE: Fix sending of SIGKILL

3.16.63-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: "Eric W. Biederman" <ebiederm@...ssion.com>

commit 0ab93e9c99f8208c0a1a7b7170c827936268c996 upstream.

The genweq_add_file and genwqe_del_file by caching current without
using reference counting embed the assumption that a file descriptor
will never be passed from one process to another.  It even embeds the
assumption that the the thread that opened the file will be in
existence when the process terminates.   Neither of which are
guaranteed to be true.

Therefore replace caching the task_struct of the opener with
pid of the openers thread group id.  All the knowledge of the
opener is used for is as the target of SIGKILL and a SIGKILL
will kill the entire process group.

Rename genwqe_force_sig to genwqe_terminate, remove it's unncessary
signal argument, update it's ownly caller, and use kill_pid
instead of force_sig.

The work force_sig does in changing signal handling state is not
relevant to SIGKILL sent as SEND_SIG_PRIV.  The exact same processess
will be killed just with less work, and less confusion.  The work done
by force_sig is really only needed for handling syncrhonous
exceptions.

It will still be possible to cause genwqe_device_remove to wait
8 seconds by passing a file descriptor to another process but
the possible user after free is fixed.

Fixes: eaf4722d4645 ("GenWQE Character device and DDCB queue")
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Frank Haverkamp <haver@...ux.vnet.ibm.com>
Cc: Joerg-Stephan Vogt <jsvogt@...ibm.com>
Cc: Michael Jung <mijung@....net>
Cc: Michael Ruettger <michael@...ra.de>
Cc: Kleber Sacilotto de Souza <klebers@...ux.vnet.ibm.com>
Cc: Sebastian Ott <sebott@...ux.vnet.ibm.com>
Cc: Eberhard S. Amann <esa@...ux.vnet.ibm.com>
Cc: Gabriel Krisman Bertazi <krisman@...ux.vnet.ibm.com>
Cc: Guilherme G. Piccoli <gpiccoli@...ux.vnet.ibm.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 drivers/misc/genwqe/card_base.h | 2 +-
 drivers/misc/genwqe/card_dev.c  | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

--- a/drivers/misc/genwqe/card_base.h
+++ b/drivers/misc/genwqe/card_base.h
@@ -401,7 +401,7 @@ struct genwqe_file {
 	struct file *filp;
 
 	struct fasync_struct *async_queue;
-	struct task_struct *owner;
+	struct pid *opener;
 	struct list_head list;		/* entry in list of open files */
 
 	spinlock_t map_lock;		/* lock for dma_mappings */
--- a/drivers/misc/genwqe/card_dev.c
+++ b/drivers/misc/genwqe/card_dev.c
@@ -52,7 +52,7 @@ static void genwqe_add_file(struct genwq
 {
 	unsigned long flags;
 
-	cfile->owner = current;
+	cfile->opener = get_pid(task_tgid(current));
 	spin_lock_irqsave(&cd->file_lock, flags);
 	list_add(&cfile->list, &cd->file_list);
 	spin_unlock_irqrestore(&cd->file_lock, flags);
@@ -65,6 +65,7 @@ static int genwqe_del_file(struct genwqe
 	spin_lock_irqsave(&cd->file_lock, flags);
 	list_del(&cfile->list);
 	spin_unlock_irqrestore(&cd->file_lock, flags);
+	put_pid(cfile->opener);
 
 	return 0;
 }
@@ -275,7 +276,7 @@ static int genwqe_kill_fasync(struct gen
 	return files;
 }
 
-static int genwqe_force_sig(struct genwqe_dev *cd, int sig)
+static int genwqe_terminate(struct genwqe_dev *cd)
 {
 	unsigned int files = 0;
 	unsigned long flags;
@@ -283,7 +284,7 @@ static int genwqe_force_sig(struct genwq
 
 	spin_lock_irqsave(&cd->file_lock, flags);
 	list_for_each_entry(cfile, &cd->file_list, list) {
-		force_sig(sig, cfile->owner);
+		kill_pid(cfile->opener, SIGKILL, 1);
 		files++;
 	}
 	spin_unlock_irqrestore(&cd->file_lock, flags);
@@ -1346,7 +1347,7 @@ static int genwqe_inform_and_stop_proces
 		dev_warn(&pci_dev->dev,
 			 "[%s] send SIGKILL and wait ...\n", __func__);
 
-		rc = genwqe_force_sig(cd, SIGKILL); /* force terminate */
+		rc = genwqe_terminate(cd);
 		if (rc) {
 			/* Give kill_timout more seconds to end processes */
 			for (i = 0; (i < genwqe_kill_timeout) &&