lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20061201234826.GA9511@oleg>
Date:	Sat, 2 Dec 2006 02:48:26 +0300
From:	Oleg Nesterov <oleg@...sign.ru>
To:	Andrew Morton <akpm@...l.org>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] introduce put_pid_rcu() to fix unsafe put_pid(vc->vt_pid)

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@...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@...de.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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ