All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Andrew Morton <akpm@osdl.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] introduce put_pid_rcu() to fix unsafe put_pid(vc->vt_pid)
Date: Sat, 2 Dec 2006 02:48:26 +0300	[thread overview]
Message-ID: <20061201234826.GA9511@oleg> (raw)

Compile tested.

drivers/char/vt_ioctl.c changes vc->vt_pid doing

	put_pid(xchg(&vc->vt_pid, ...));

This is unsafe, put_pid() can actually free the memory while vc->vt_pid is
still used by kill_pid(vc->vt_pid).

Add a new helper, put_pid_rcu(), which frees "struct pid" via rcu callback
and convert vt_ioctl.c to use it.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

 include/linux/pid.h     |    2 ++
 kernel/pid.c            |   18 ++++++++++++++++++
 drivers/char/vt_ioctl.c |   22 ++++++++++++++++++----
 3 files changed, 38 insertions(+), 4 deletions(-)

--- 19-rc6/include/linux/pid.h~vt_pid	2006-10-22 18:24:02.000000000 +0400
+++ 19-rc6/include/linux/pid.h	2006-12-02 01:38:59.000000000 +0300
@@ -64,6 +64,8 @@ static inline struct pid *get_pid(struct
 }
 
 extern void FASTCALL(put_pid(struct pid *pid));
+extern void put_pid_rcu(struct pid *pid);
+
 extern struct task_struct *FASTCALL(pid_task(struct pid *pid, enum pid_type));
 extern struct task_struct *FASTCALL(get_pid_task(struct pid *pid,
 						enum pid_type));
--- 19-rc6/kernel/pid.c~vt_pid	2006-10-22 18:24:03.000000000 +0400
+++ 19-rc6/kernel/pid.c	2006-12-02 01:35:15.000000000 +0300
@@ -196,6 +196,24 @@ fastcall void free_pid(struct pid *pid)
 	call_rcu(&pid->rcu, delayed_put_pid);
 }
 
+static void delayed_free_pid(struct rcu_head *rhp)
+{
+	struct pid *pid = container_of(rhp, struct pid, rcu);
+	kmem_cache_free(pid_cachep, pid);
+}
+
+void put_pid_rcu(struct pid *pid)
+{
+	if (!pid)
+		return;
+	/*
+	 * ->count can't go to 0 unless delayed_put_pid() was already
+	 * called (RCU owns a reference), so we can re-use pid->rcu.
+	 */
+	if (atomic_dec_and_test(&pid->count))
+		call_rcu(&pid->rcu, delayed_free_pid);
+}
+
 struct pid *alloc_pid(void)
 {
 	struct pid *pid;
--- 19-rc6/drivers/char/vt_ioctl.c~vt_pid	2006-10-22 18:23:58.000000000 +0400
+++ 19-rc6/drivers/char/vt_ioctl.c	2006-12-02 02:44:48.000000000 +0300
@@ -358,6 +358,20 @@ do_unimap_ioctl(int cmd, struct unimapde
 	return 0;
 }
 
+static inline void set_vt_pid(struct vc_data *vc, struct pid *pid)
+{
+	put_pid_rcu(xchg(&vc->vt_pid, pid));
+}
+
+static int kill_vt_pid(struct vc_data *vc, int sig)
+{
+	int ret;
+	rcu_read_lock();
+	ret = kill_pid(rcu_dereference(vc->vt_pid), sig, 1);
+	rcu_read_unlock();
+	return ret;
+}
+
 /*
  * We handle the console-specific ioctl's here.  We allow the
  * capability to modify any console, not just the fg_console. 
@@ -672,7 +686,7 @@ int vt_ioctl(struct tty_struct *tty, str
 		vc->vt_mode = tmp;
 		/* the frsig is ignored, so we set it to 0 */
 		vc->vt_mode.frsig = 0;
-		put_pid(xchg(&vc->vt_pid, get_pid(task_pid(current))));
+		set_vt_pid(vc, get_pid(task_pid(current)));
 		/* no switch is required -- saw@shade.msu.ru */
 		vc->vt_newvt = -1;
 		release_console_sem();
@@ -1063,7 +1077,7 @@ void reset_vc(struct vc_data *vc)
 	vc->vt_mode.relsig = 0;
 	vc->vt_mode.acqsig = 0;
 	vc->vt_mode.frsig = 0;
-	put_pid(xchg(&vc->vt_pid, NULL));
+	set_vt_pid(vc, NULL);
 	vc->vt_newvt = -1;
 	if (!in_interrupt())    /* Via keyboard.c:SAK() - akpm */
 		reset_palette(vc);
@@ -1114,7 +1128,7 @@ static void complete_change_console(stru
 		 * tell us if the process has gone or something else
 		 * is awry
 		 */
-		if (kill_pid(vc->vt_pid, vc->vt_mode.acqsig, 1) != 0) {
+		if (kill_vt_pid(vc, vc->vt_mode.acqsig) != 0) {
 		/*
 		 * The controlling process has died, so we revert back to
 		 * normal operation. In this case, we'll also change back
@@ -1174,7 +1188,7 @@ void change_console(struct vc_data *new_
 		 * tell us if the process has gone or something else
 		 * is awry
 		 */
-		if (kill_pid(vc->vt_pid, vc->vt_mode.relsig, 1) == 0) {
+		if (kill_vt_pid(vc, vc->vt_mode.relsig) == 0) {
 			/*
 			 * It worked. Mark the vt to switch to and
 			 * return. The process needs to send us a


             reply	other threads:[~2006-12-01 23:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-01 23:48 Oleg Nesterov [this message]
2006-12-03 21:02 ` [PATCH] introduce put_pid_rcu() to fix unsafe put_pid(vc->vt_pid) Andrew Morton
2006-12-03 21:29   ` Oleg Nesterov
2006-12-03 23:18     ` Eric W. Biederman
2006-12-03 23:51       ` Oleg Nesterov

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=20061201234826.GA9511@oleg \
    --to=oleg@tv-sign.ru \
    --cc=akpm@osdl.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.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.