All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Kirill Tkhai <ktkhai@parallels.com>,
	Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	Vladimir Davydov <vdavydov@parallels.com>,
	Kirill Tkhai <tkhai@yandex.ru>, Christoph Lameter <cl@linux.com>
Subject: [PATCH 3/3] introduce task_rcu_dereference()
Date: Mon, 27 Oct 2014 20:54:46 +0100	[thread overview]
Message-ID: <20141027195446.GD11736@redhat.com> (raw)
In-Reply-To: <20141027195339.GA11736@redhat.com>

task_struct is only protected by RCU if it was found on a RCU protected
list (say, for_each_process() or find_task_by_vpid()).

And as Kirill pointed out rq->curr isn't protected by RCU, the scheduler
drops the (potentially) last reference without RCU gp, this means that we
need to fix the code which uses foreign_rq->curr under rcu_read_lock().

Add a new helper which can be used to dereferene rq->curr or any other
pointer to task_struct assuming that it should be cleared or updated
before the final put_task_struct(). It returns non-NULL only if this
task can't go away before rcu_read_unlock().

Suggested-by: Kirill Tkhai <ktkhai@parallels.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/sched.h |    1 +
 kernel/exit.c         |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 857ba40..0ba420e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2300,6 +2300,7 @@ extern void block_all_signals(int (*notifier)(void *priv), void *priv,
 			      sigset_t *mask);
 extern void unblock_all_signals(void);
 extern void release_task(struct task_struct * p);
+extern struct task_struct *task_rcu_dereference(struct task_struct **ptask);
 extern int send_sig_info(int, struct siginfo *, struct task_struct *);
 extern int force_sigsegv(int, struct task_struct *);
 extern int force_sig_info(int, struct siginfo *, struct task_struct *);
diff --git a/kernel/exit.c b/kernel/exit.c
index 32c58f7..d8b95c2 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -213,6 +213,55 @@ repeat:
 		goto repeat;
 }
 
+struct task_struct *task_rcu_dereference(struct task_struct **ptask)
+{
+	struct task_struct *task;
+	struct sighand_struct *sighand;
+
+	/*
+	 * We need to verify that release_task() was not called and thus
+	 * delayed_put_task_struct() can't run and drop the last reference
+	 * before rcu_read_unlock(). We check task->sighand != NULL, but
+	 * we can read the already freed and reused memory.
+	 */
+ retry:
+	task = rcu_dereference(*ptask);
+	if (!task)
+		return NULL;
+
+	probe_slab_address(&task->sighand, sighand);
+	/*
+	 * Pairs with atomic_dec_and_test(usage) in put_task_struct(task).
+	 * If this task was already freed we can not miss the preceding
+	 * update of this pointer.
+	 */
+	smp_rmb();
+	if (unlikely(task != ACCESS_ONCE(*ptask)))
+		goto retry;
+
+	/*
+	 * Either this is the same task and we can trust sighand != NULL, or
+	 * its memory was re-instantiated as another instance of task_struct.
+	 * In the latter case the new task can not go away until another rcu
+	 * gp pass, so the only problem is that sighand == NULL can be false
+	 * positive but we can pretend we got this NULL before it was freed.
+	 */
+	if (sighand)
+		return task;
+
+	/*
+	 * We could even eliminate the false positive mentioned above:
+	 *
+	 *	probe_slab_address(&task->sighand, sighand);
+	 *	if (sighand)
+	 *		goto retry;
+	 *
+	 * if sighand != NULL because we read the freed memory we should
+	 * see the new pointer, otherwise we will likely return this task.
+	 */
+	return NULL;
+}
+
 /*
  * This checks not only the pgrp, but falls back on the pid if no
  * satisfactory pgrp is found. I dunno - gdb doesn't work correctly
-- 
1.5.5.1



  parent reply	other threads:[~2014-10-27 18:58 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-22  7:17 [PATCH v4] sched/numa: fix unsafe get_task_struct() in task_numa_assign() Kirill Tkhai
2014-10-22 21:30 ` introduce task_rcu_dereference? Oleg Nesterov
2014-10-22 22:23   ` Oleg Nesterov
2014-10-23 18:15     ` Oleg Nesterov
2014-10-23  8:10   ` Kirill Tkhai
2014-10-23 18:18     ` Oleg Nesterov
2014-10-24  7:51       ` Kirill Tkhai
2014-10-27 19:53 ` [PATCH 0/3] introduce task_rcu_dereference() Oleg Nesterov
2014-10-27 19:54   ` [PATCH 1/3] probe_kernel_address() can use __probe_kernel_read() Oleg Nesterov
2014-10-27 19:54   ` [PATCH 2/3] introduce probe_slab_address() Oleg Nesterov
2014-10-27 19:21     ` Christoph Lameter
2014-10-28  5:44     ` Kirill Tkhai
2014-10-28  5:48       ` Kirill Tkhai
2014-10-28 15:01       ` Peter Zijlstra
2014-10-28 17:56         ` Kirill Tkhai
2014-10-28 18:00           ` Kirill Tkhai
2014-10-28 19:55           ` Oleg Nesterov
2014-10-28 20:12             ` Oleg Nesterov
2014-10-29  5:10               ` Kirill Tkhai
2014-10-27 19:54   ` Oleg Nesterov [this message]
2014-10-28  6:22     ` [PATCH 3/3] introduce task_rcu_dereference() Kirill Tkhai
2016-05-18 17:02     ` Peter Zijlstra
2016-05-18 18:23       ` Oleg Nesterov
2016-05-18 19:10         ` Peter Zijlstra
2016-05-18 19:57           ` Oleg Nesterov
2016-05-26 11:34             ` Peter Zijlstra
2016-06-03 10:49             ` [tip:sched/core] sched/fair: Use task_rcu_dereference() tip-bot for Oleg Nesterov
2016-06-03 10:48       ` [tip:sched/core] sched/api: Introduce task_rcu_dereference() and try_get_task_struct() tip-bot for Oleg Nesterov
2014-10-28 11:02 ` [tip:sched/core] sched/numa: Fix unsafe get_task_struct() in task_numa_assign() tip-bot for Kirill Tkhai
2014-11-08  3:48 ` [PATCH v4] sched/numa: fix " Sasha Levin
2014-11-09 14:07   ` Kirill Tkhai
2014-11-10 10:03     ` Peter Zijlstra
2014-11-10 15:48       ` Sasha Levin
2014-11-10 16:01         ` Peter Zijlstra
2014-11-16  9:50       ` [tip:sched/urgent] sched/numa: Avoid selecting oneself as swap target tip-bot for Peter Zijlstra
2014-11-10 16:03   ` [PATCH v4] sched/numa: fix unsafe get_task_struct() in task_numa_assign() Peter Zijlstra
2014-11-10 16:09     ` Sasha Levin
2014-11-10 16:16       ` Peter Zijlstra
2014-11-10 16:10     ` Kirill Tkhai
2014-11-10 16:36       ` Kirill Tkhai
2014-11-10 16:44         ` Sasha Levin
2014-11-10 20:01           ` Kirill Tkhai
2014-11-12  9:49             ` Kirill Tkhai
2014-11-15  2:38     ` Sasha Levin
2014-11-18 17:30       ` Sasha Levin

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=20141027195446.GD11736@redhat.com \
    --to=oleg@redhat.com \
    --cc=cl@linux.com \
    --cc=ktkhai@parallels.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tkhai@yandex.ru \
    --cc=vdavydov@parallels.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.