All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Dave Jones <davej@redhat.com>,
	Tejun Heo <tj@kernel.org>, Li Zefan <lizefan@huawei.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Zhao Hongjiang <zhaohongjiang@huawei.com>
Subject: [PATCH 3.4 17/39] exec: do not abuse ->cred_guard_mutex in threadgroup_lock()
Date: Tue, 26 Nov 2013 16:56:41 -0800	[thread overview]
Message-ID: <20131127005620.235386186@linuxfoundation.org> (raw)
In-Reply-To: <20131127005619.011763867@linuxfoundation.org>

3.4-stable review patch.  If anyone has any objections, please let me know.

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

From: Oleg Nesterov <oleg@redhat.com>

commit e56fb2874015370e3b7f8d85051f6dce26051df9 upstream.

threadgroup_lock() takes signal->cred_guard_mutex to ensure that
thread_group_leader() is stable.  This doesn't look nice, the scope of
this lock in do_execve() is huge.

And as Dave pointed out this can lead to deadlock, we have the
following dependencies:

	do_execve:		cred_guard_mutex -> i_mutex
	cgroup_mount:		i_mutex -> cgroup_mutex
	attach_task_by_pid:	cgroup_mutex -> cred_guard_mutex

Change de_thread() to take threadgroup_change_begin() around the
switch-the-leader code and change threadgroup_lock() to avoid
->cred_guard_mutex.

Note that de_thread() can't sleep with ->group_rwsem held, this can
obviously deadlock with the exiting leader if the writer is active, so it
does threadgroup_change_end() before schedule().

Reported-by: Dave Jones <davej@redhat.com>
Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[ zhj: adjust context ]
Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/exec.c             |    3 +++
 include/linux/sched.h |   18 ++++--------------
 2 files changed, 7 insertions(+), 14 deletions(-)

--- a/fs/exec.c
+++ b/fs/exec.c
@@ -909,11 +909,13 @@ static int de_thread(struct task_struct
 
 		sig->notify_count = -1;	/* for exit_notify() */
 		for (;;) {
+			threadgroup_change_begin(tsk);
 			write_lock_irq(&tasklist_lock);
 			if (likely(leader->exit_state))
 				break;
 			__set_current_state(TASK_UNINTERRUPTIBLE);
 			write_unlock_irq(&tasklist_lock);
+			threadgroup_change_end(tsk);
 			schedule();
 		}
 
@@ -969,6 +971,7 @@ static int de_thread(struct task_struct
 		if (unlikely(leader->ptrace))
 			__wake_up_parent(leader, leader->parent);
 		write_unlock_irq(&tasklist_lock);
+		threadgroup_change_end(tsk);
 
 		release_task(leader);
 	}
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2466,27 +2466,18 @@ static inline void threadgroup_change_en
  *
  * Lock the threadgroup @tsk belongs to.  No new task is allowed to enter
  * and member tasks aren't allowed to exit (as indicated by PF_EXITING) or
- * perform exec.  This is useful for cases where the threadgroup needs to
- * stay stable across blockable operations.
+ * change ->group_leader/pid.  This is useful for cases where the threadgroup
+ * needs to stay stable across blockable operations.
  *
  * fork and exit paths explicitly call threadgroup_change_{begin|end}() for
  * synchronization.  While held, no new task will be added to threadgroup
  * and no existing live task will have its PF_EXITING set.
  *
- * During exec, a task goes and puts its thread group through unusual
- * changes.  After de-threading, exclusive access is assumed to resources
- * which are usually shared by tasks in the same group - e.g. sighand may
- * be replaced with a new one.  Also, the exec'ing task takes over group
- * leader role including its pid.  Exclude these changes while locked by
- * grabbing cred_guard_mutex which is used to synchronize exec path.
+ * de_thread() does threadgroup_change_{begin|end}() when a non-leader
+ * sub-thread becomes a new leader.
  */
 static inline void threadgroup_lock(struct task_struct *tsk)
 {
-	/*
-	 * exec uses exit for de-threading nesting group_rwsem inside
-	 * cred_guard_mutex. Grab cred_guard_mutex first.
-	 */
-	mutex_lock(&tsk->signal->cred_guard_mutex);
 	down_write(&tsk->signal->group_rwsem);
 }
 
@@ -2499,7 +2490,6 @@ static inline void threadgroup_lock(stru
 static inline void threadgroup_unlock(struct task_struct *tsk)
 {
 	up_write(&tsk->signal->group_rwsem);
-	mutex_unlock(&tsk->signal->cred_guard_mutex);
 }
 #else
 static inline void threadgroup_change_begin(struct task_struct *tsk) {}



  parent reply	other threads:[~2013-11-27  2:04 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-27  0:56 [PATCH 3.4 00/39] 3.4.71-stable review Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 01/39] vfs,proc: guarantee unique inodes in /proc Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 02/39] nfs: dont allow nfs_find_actor to match inodes of the wrong type Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 03/39] libertas: potential oops in debugfs Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 04/39] aacraid: prevent invalid pointer dereference Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 05/39] ACPICA: Interpreter: Fix Store() when implicit conversion is not possible Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 06/39] ACPICA: DeRefOf operator: Update to fully resolve FieldUnit and BufferField refs Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 07/39] ACPICA: Return error if DerefOf resolves to a null package element Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 08/39] ACPICA: Fix for a Store->ArgX when ArgX contains a reference to a field Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 09/39] USB: mos7840: fix tiocmget error handling Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 10/39] crypto: ansi_cprng - Fix off by one error in non-block size request Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 11/39] can: c_can: Fix RX message handling, handle lost message before EOB Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 12/39] 8139cp: re-enable interrupts after tx timeout Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 14/39] SUNRPC handle EKEYEXPIRED in call_refreshresult Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 15/39] SUNRPC: dont map EKEYEXPIRED to EACCES " Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 16/39] Nest rename_lock inside vfsmount_lock Greg Kroah-Hartman
2013-11-27  0:56 ` Greg Kroah-Hartman [this message]
2013-11-27  0:56 ` [PATCH 3.4 18/39] include/linux/fs.h: disable preempt when acquire i_size_seqcount write lock Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 19/39] perf/ftrace: Fix paranoid level for enabling function tracer Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 20/39] rt2x00: check if device is still available on rt2x00mac_flush() Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 21/39] Revert "ima: policy for RAMFS" Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 22/39] exec/ptrace: fix get_dumpable() incorrect tests Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 23/39] ALSA: 6fire: Fix probe of multiple cards Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 24/39] ALSA: msnd: Avoid duplicated driver name Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 25/39] NFSv4: Fix a use-after-free situation in _nfs4_proc_getlk() Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 26/39] nfsd: split up nfsd_setattr Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 27/39] nfsd: make sure to balance get/put_write_access Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 28/39] x86/microcode/amd: Tone down printk(), dont treat a missing firmware file as an error Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 29/39] hwmon: (lm90) Fix max6696 alarm handling Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 30/39] block: fix race between request completion and timeout handling Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 31/39] block: fix a probe argument to blk_register_region Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 32/39] block: properly stack underlying max_segment_size to DM device Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 33/39] powerpc/vio: use strcpy in modalias_show Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 34/39] powerpc/powernv: Add PE to its own PELTV Greg Kroah-Hartman
2013-11-27  0:56 ` [PATCH 3.4 35/39] powerpc/signals: Mark VSX not saved with small contexts Greg Kroah-Hartman
2013-11-27  0:57 ` [PATCH 3.4 36/39] SUNRPC: Fix a data corruption issue when retransmitting RPC calls Greg Kroah-Hartman
2013-11-27  0:57 ` [PATCH 3.4 37/39] rt2800usb: slow down TX status polling Greg Kroah-Hartman
2013-11-27  0:57 ` [PATCH 3.4 38/39] configfs: fix race between dentry put and lookup Greg Kroah-Hartman
2013-11-27  0:57 ` [PATCH 3.4 39/39] cris: media platform drivers: fix build Greg Kroah-Hartman
2013-11-27  4:14 ` [PATCH 3.4 00/39] 3.4.71-stable review Guenter Roeck
2013-11-27 22:29 ` Shuah Khan
2013-11-28 10:54 ` Satoru Takeuchi

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=20131127005620.235386186@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=oleg@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=zhaohongjiang@huawei.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.