linux-kernel - [PATCH 1/1] kthread: fix possible infinite wait for parking when kthread exits meanwhile

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-next>] [day] [month] [year] [list]

Message-Id: <20161025110525.9100-1-roman.penyaev@profitbricks.com>
Date:   Tue, 25 Oct 2016 13:05:25 +0200
From:   Roman Pen <roman.penyaev@...fitbricks.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     Roman Pen <roman.penyaev@...fitbricks.com>,
        Andy Lutomirski <luto@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Tejun Heo <tj@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: [PATCH 1/1] kthread: fix possible infinite wait for parking when kthread exits meanwhile

The patch handles the case, when someone waits on parked completion but
kthread exits meanwhile.  To avoid infinite wait the waiter has to spin
once more in a loop and simply try to get an alive kthread.  If the
kthread has been died, put_kthread_cb() wakes up possible waiter when
kthread->vfork_done is already NULL, so next attempt to grab alive
kthread pointer will fail.

Signed-off-by: Roman Pen <roman.penyaev@...fitbricks.com>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Tejun Heo <tj@...nel.org>
Cc: linux-kernel@...r.kernel.org
---
 kernel/kthread.c | 41 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/kernel/kthread.c b/kernel/kthread.c
index e8adc10556e0..a001c1ec489b 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -77,6 +77,12 @@ static void put_kthread_cb(struct callback_head *work)
 	struct kthread *kthread;
 
 	kthread = container_of(work, struct kthread, put_work);
+	/*
+	 * Kick out possible waiter on a parked completion before
+	 * ref put.  That will force them to spin in a loop once
+	 * more and eventually get the NULL kthread pointer.
+	 */
+	complete(&kthread->parked);
 	put_kthread(kthread);
 }
 
@@ -449,6 +455,11 @@ static void __kthread_unpark(struct task_struct *k, struct kthread *kthread)
 	}
 }
 
+static bool __kthread_isparked(struct kthread *kthread)
+{
+	return test_bit(KTHREAD_IS_PARKED, &kthread->flags);
+}
+
 /**
  * kthread_unpark - unpark a thread created by kthread_create().
  * @k:		thread created by kthread_create().
@@ -479,23 +490,43 @@ EXPORT_SYMBOL_GPL(kthread_unpark);
  *
  * Returns 0 if the thread is parked, -ENOSYS if the thread exited.
  * If called by the kthread itself just the park bit is set.
+ *
+ * BEWARE: The caller is responsible for ensuring the validity of @k when
+ *         calling this function.
+ *
+ * BEWARE: Only one simultaneous caller is possible.  Others will hang
+ *         forever.  You have been warned.
  */
 int kthread_park(struct task_struct *k)
 {
-	struct kthread *kthread = to_live_kthread_and_get(k);
+	struct kthread *kthread;
 	int ret = -ENOSYS;
 
-	if (kthread) {
-		if (!test_bit(KTHREAD_IS_PARKED, &kthread->flags)) {
+	/*
+	 * Here we try to be careful and handle the case, when kthread
+	 * is going to die and will never park.  In that particular case
+	 * put_kthread_cb() is called when kthread->vfork_done is already
+	 * NULL.  put_kthread_cb() does the last completion on kthread->parked,
+	 * thus we will spin once more and next attempt to get an alive
+	 * kthread will fail.
+	 */
+	do {
+		kthread = to_live_kthread_and_get(k);
+		if (!kthread)
+			break;
+		if (!__kthread_isparked(kthread)) {
 			set_bit(KTHREAD_SHOULD_PARK, &kthread->flags);
 			if (k != current) {
 				wake_up_process(k);
 				wait_for_completion(&kthread->parked);
 			}
 		}
+		if (k == current || __kthread_isparked(kthread))
+			/* The way out */
+			ret = 0;
 		put_kthread(kthread);
-		ret = 0;
-	}
+	} while (ret);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kthread_park);
-- 
2.9.3