All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Hugh Dickins <hugh@veritas.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joe Malicki <jmalicki@metacarta.com>,
	Michael Itz <mitz@metacarta.com>,
	Kenneth Baker <bakerk@metacarta.com>,
	Chris Wright <chrisw@sous-sol.org>,
	David Howells <dhowells@redhat.com>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Roland McGrath <roland@redhat.com>
Subject: [PATCH 2/2] check_unsafe_exec: s/lock_task_sighand/rcu_read_lock/
Date: Fri, 24 Apr 2009 01:02:45 +0200	[thread overview]
Message-ID: <20090423230245.GB31302@redhat.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0904212031120.10256@blonde.anvils>

write_lock(&current->fs->lock) guarantees we can't wrongly miss
LSM_UNSAFE_SHARE, this is what we care about. Use rcu_read_lock()
instead of ->siglock to iterate over the sub-threads. We must see
all CLONE_THREAD|CLONE_FS threads which didn't pass exit_fs(), it
takes fs->lock too.

With or without this patch we can miss the freshly cloned thread
and set LSM_UNSAFE_SHARE, we don't care.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>

--- PTRACE/fs/exec.c~2_NO_SIGLOCK	2009-04-24 00:01:53.000000000 +0200
+++ PTRACE/fs/exec.c	2009-04-24 00:36:22.000000000 +0200
@@ -1060,7 +1060,6 @@ EXPORT_SYMBOL(install_exec_creds);
 int check_unsafe_exec(struct linux_binprm *bprm)
 {
 	struct task_struct *p = current, *t;
-	unsigned long flags;
 	unsigned n_fs;
 	int res = 0;
 
@@ -1068,11 +1067,12 @@ int check_unsafe_exec(struct linux_binpr
 
 	n_fs = 1;
 	write_lock(&p->fs->lock);
-	lock_task_sighand(p, &flags);
+	rcu_read_lock();
 	for (t = next_thread(p); t != p; t = next_thread(t)) {
 		if (t->fs == p->fs)
 			n_fs++;
 	}
+	rcu_read_lock();
 
 	if (p->fs->users > n_fs) {
 		bprm->unsafe |= LSM_UNSAFE_SHARE;
@@ -1083,8 +1083,6 @@ int check_unsafe_exec(struct linux_binpr
 			res = 1;
 		}
 	}
-
-	unlock_task_sighand(p, &flags);
 	write_unlock(&p->fs->lock);
 
 	return res;


  parent reply	other threads:[~2009-04-23 23:09 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-28 23:16 [PATCH 1/4] compat_do_execve should unshare_files Hugh Dickins
2009-03-28 23:20 ` [PATCH 2/4] fix setuid sometimes doesn't Hugh Dickins
2009-03-29  0:53   ` Q: check_unsafe_exec() races (Was: [PATCH 2/4] fix setuid sometimes doesn't) Oleg Nesterov
2009-03-29  4:10     ` Al Viro
2009-03-29  4:14       ` Al Viro
2009-03-29  4:52       ` Oleg Nesterov
2009-03-29  5:55         ` Al Viro
2009-03-29  6:01           ` Al Viro
2009-03-29 21:36             ` Oleg Nesterov
2009-03-29 22:20               ` Al Viro
2009-03-29 23:56                 ` Oleg Nesterov
2009-03-30  0:03                   ` Oleg Nesterov
2009-03-30  1:08                     ` Al Viro
2009-03-30  1:13                       ` Al Viro
2009-03-30  1:36                         ` Oleg Nesterov
2009-03-30  1:40                           ` Oleg Nesterov
2009-03-30 12:31                             ` Al Viro
2009-03-30 14:32                               ` Hugh Dickins
2009-03-31  6:16                                 ` Al Viro
2009-04-01  0:28                                   ` Hugh Dickins
2009-04-01  2:38                                     ` Al Viro
2009-04-01  3:03                                       ` Al Viro
2009-04-01 11:25                                         ` Hugh Dickins
2009-04-06 15:31                                         ` Oleg Nesterov
2009-04-19 16:30                                           ` Hugh Dickins
2009-04-21 16:10                                             ` Oleg Nesterov
2009-04-21 16:31                                               ` Linus Torvalds
2009-04-21 17:15                                                 ` Oleg Nesterov
2009-04-21 17:35                                                   ` Linus Torvalds
2009-04-21 19:39                                                     ` Hugh Dickins
2009-04-23 23:01                                                       ` [PATCH 1/2] do_execve() must not clear fs->in_exec if it was set by another thread Oleg Nesterov
2009-04-23 23:18                                                         ` Roland McGrath
2009-04-23 23:31                                                         ` Al Viro
2009-04-24 11:57                                                           ` [PATCH 3/2] check_unsafe_exec: rcu_read_unlock Hugh Dickins
2009-04-24 14:34                                                             ` Oleg Nesterov
2009-04-24  4:20                                                         ` [PATCH 1/2] do_execve() must not clear fs->in_exec if it was set by another thread Hugh Dickins
2009-04-23 23:02                                                       ` Oleg Nesterov [this message]
2009-04-23 23:18                                                         ` [PATCH 2/2] check_unsafe_exec: s/lock_task_sighand/rcu_read_lock/ Roland McGrath
2009-04-24  4:29                                                         ` Hugh Dickins
2009-04-01 11:18                                       ` Q: check_unsafe_exec() races (Was: [PATCH 2/4] fix setuid sometimes doesn't) Hugh Dickins
2009-04-06 15:51                                       ` Oleg Nesterov
2009-04-19 16:44                                         ` Hugh Dickins
2009-04-21 16:39                                           ` Oleg Nesterov
2009-03-30 23:45                               ` Serge E. Hallyn
2009-03-31  6:19                                 ` Al Viro
2009-03-28 23:21 ` [PATCH 3/4] fix setuid sometimes wouldn't Hugh Dickins
2009-03-29 11:19   ` Alexey Dobriyan
2009-03-29 21:48     ` Oleg Nesterov
2009-03-29 22:37       ` Al Viro
2009-03-28 23:23 ` [PATCH 4/4] Annotate struct fs_struct's usage count restriction Hugh Dickins

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=20090423230245.GB31302@redhat.com \
    --to=oleg@redhat.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=bakerk@metacarta.com \
    --cc=chrisw@sous-sol.org \
    --cc=dhowells@redhat.com \
    --cc=gregkh@suse.de \
    --cc=hugh@veritas.com \
    --cc=jmalicki@metacarta.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mitz@metacarta.com \
    --cc=roland@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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.