mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Roman Pen <roman.penyaev@profitbricks.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Roman Pen <roman.penyaev@profitbricks.com>,
	Andy Lutomirski <luto@kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/2] workqueue: ignore dead tasks in a workqueue sleep hook
Date: Mon, 24 Oct 2016 18:08:14 +0200	[thread overview]
Message-ID: <20161024160814.3126-2-roman.penyaev@profitbricks.com> (raw)
In-Reply-To: <20161024160814.3126-1-roman.penyaev@profitbricks.com>

If panic_on_oops is not set and oops happens inside workqueue kthread,
kernel kills this kthread.  Current patch fixes recursive GPF which
happens when wq_worker_sleeping() function unconditionally accesses
the NULL kthread->vfork_done ptr trhu kthread_data() -> to_kthread().

The stack is the following:

[<ffffffff81397f75>] dump_stack+0x68/0x93
[<ffffffff8106954b>] ? do_exit+0x7ab/0xc10
[<ffffffff8108fd73>] __schedule_bug+0x83/0xe0
[<ffffffff81716d5a>] __schedule+0x7ea/0xba0
[<ffffffff810c864f>] ? vprintk_default+0x1f/0x30
[<ffffffff8116a63c>] ? printk+0x48/0x50
[<ffffffff81717150>] schedule+0x40/0x90
[<ffffffff8106976a>] do_exit+0x9ca/0xc10
[<ffffffff810c8e3d>] ? kmsg_dump+0x11d/0x190
[<ffffffff810c8d37>] ? kmsg_dump+0x17/0x190
[<ffffffff81021ee9>] oops_end+0x99/0xd0
[<ffffffff81052da5>] no_context+0x185/0x3e0
[<ffffffff81053083>] __bad_area_nosemaphore+0x83/0x1c0
[<ffffffff810c820e>] ? vprintk_emit+0x25e/0x530
[<ffffffff810531d4>] bad_area_nosemaphore+0x14/0x20
[<ffffffff8105355c>] __do_page_fault+0xac/0x570
[<ffffffff810c66fe>] ? console_trylock+0x1e/0xe0
[<ffffffff81002036>] ? trace_hardirqs_off_thunk+0x1a/0x1c
[<ffffffff81053a2c>] do_page_fault+0xc/0x10
[<ffffffff8171f812>] page_fault+0x22/0x30
[<ffffffff81089bc3>] ? kthread_data+0x33/0x40
[<ffffffff8108427e>] ? wq_worker_sleeping+0xe/0x80
[<ffffffff817169eb>] __schedule+0x47b/0xba0
[<ffffffff81717150>] schedule+0x40/0x90
[<ffffffff8106957d>] do_exit+0x7dd/0xc10
[<ffffffff81021ee9>] oops_end+0x99/0xd0

kthread->vfork_done is zeroed out on the following path:

    do_exit()
    exit_mm()
    mm_release()
    complete_vfork_done()

In order to fix a bug dead tasks must be ignored.

Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org
---
v2:
  o put a task->state check directly into a wq_worker_sleeping() function
    instead of changing the __schedule().

 kernel/workqueue.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 9dc7ac5..b19dcb6 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -875,9 +875,31 @@ void wq_worker_waking_up(struct task_struct *task, int cpu)
  */
 struct task_struct *wq_worker_sleeping(struct task_struct *task)
 {
-	struct worker *worker = kthread_data(task), *to_wakeup = NULL;
+	struct worker *worker, *to_wakeup = NULL;
 	struct worker_pool *pool;
 
+
+	if (task->state == TASK_DEAD)
+		/* Here we try to catch the following path before
+		 * accessing NULL kthread->vfork_done ptr thru
+		 * kthread_data():
+		 *
+		 *    oops_end()
+		 *    do_exit()
+		 *    schedule()
+		 *
+		 * If panic_on_oops is not set and oops happens on
+		 * a workqueue execution path, thread will be killed.
+		 * That is definitly sad, but not to make the situation
+		 * even worse we have to ignore dead tasks in order not
+		 * to step on zeroed out members (e.g. t->vfork_done is
+		 * already NULL on that path, since we were called by
+		 * do_exit())).
+		 */
+		return NULL;
+
+	worker = kthread_data(task);
+
 	/*
 	 * Rescuers, which may not have all the fields set up like normal
 	 * workers, also reach here, let's not access anything before
-- 
2.9.3

  reply	other threads:[~2016-10-24 16:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-24 16:08 [PATCH v2 1/2] kthread: allocate kthread structure using kmalloc Roman Pen
2016-10-24 16:08 ` Roman Pen [this message]
2016-10-24 16:40   ` [PATCH v2 2/2] workqueue: ignore dead tasks in a workqueue sleep hook Peter Zijlstra
2016-10-24 19:09     ` Roman Penyaev
2016-10-24 20:27   ` Tejun Heo
2016-10-24 16:42 ` [PATCH v2 1/2] kthread: allocate kthread structure using kmalloc Peter Zijlstra
2016-10-24 19:10   ` Roman Penyaev
2016-10-24 17:08 ` Andy Lutomirski
2016-10-24 19:37   ` Roman Penyaev

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=20161024160814.3126-2-roman.penyaev@profitbricks.com \
    --to=roman.penyaev@profitbricks.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome