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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44F367C8.9040507@watson.ibm.com>
Date:	Mon, 28 Aug 2006 18:01:44 -0400
From:	Shailabh Nagar <nagar@...son.ibm.com>
To:	Shailabh Nagar <nagar@...son.ibm.com>
CC:	Andrew Morton <akpm@...l.org>, Olaf Hering <olaf@...fle.de>,
	Linus Torvalds <torvalds@...l.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Linux v2.6.18-rc5

Cleanup allocation and freeing of tsk->delays used by delay accounting.
This solves two problems reported for delay accounting:

1. oops in __delayacct_blkio_ticks
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0608.2/1844.html

Currently tsk->delays is getting freed too early in task exit
which can cause a NULL tsk->delays to get accessed via reading
of /proc/<tgid>/stats. The patch fixes this problem by freeing
tsk->delays closer to when task_struct itself is freed up. As a result,
it also eliminates the use of tsk->delays_lock which was only being
used (inadequately) to safeguard access to tsk->delays
while a task was exiting.

2. Possible memory leak in kernel/delayacct.c
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0608.2/1389.html

The patch cleans up tsk->delays allocations after a bad fork which
was missing earlier.


The patch has been tested to fix the problems listed above
and stress tested with rapid calls to delay accounting's taskstats
command interface (which is the other path that can access the same
data, besides the /proc interface causing the oops above).


Signed-Off-By: Shailabh Nagar <nagar@...son.ibm.com>

 include/linux/delayacct.h |   10 +++++++---
 include/linux/sched.h     |    1 -
 kernel/delayacct.c        |   16 ----------------
 kernel/exit.c             |    1 -
 kernel/fork.c             |    6 ++++--
 5 files changed, 11 insertions(+), 23 deletions(-)

Index: linux-2.6.18-rc5/kernel/fork.c
===================================================================
--- linux-2.6.18-rc5.orig/kernel/fork.c	2006-08-28 11:34:27.000000000 -0400
+++ linux-2.6.18-rc5/kernel/fork.c	2006-08-28 11:34:48.000000000 -0400
@@ -117,6 +117,7 @@ void __put_task_struct(struct task_struc
 	security_task_free(tsk);
 	free_uid(tsk->user);
 	put_group_info(tsk->group_info);
+	delayacct_tsk_free(tsk);

 	if (!profile_handoff_task(tsk))
 		free_task(tsk);
@@ -1011,7 +1012,7 @@ static struct task_struct *copy_process(
 	retval = -EFAULT;
 	if (clone_flags & CLONE_PARENT_SETTID)
 		if (put_user(p->pid, parent_tidptr))
-			goto bad_fork_cleanup;
+			goto bad_fork_cleanup_delays_binfmt;

 	INIT_LIST_HEAD(&p->children);
 	INIT_LIST_HEAD(&p->sibling);
@@ -1277,7 +1278,8 @@ bad_fork_cleanup_policy:
 bad_fork_cleanup_cpuset:
 #endif
 	cpuset_exit(p);
-bad_fork_cleanup:
+bad_fork_cleanup_delays_binfmt:
+	delayacct_tsk_free(p);
 	if (p->binfmt)
 		module_put(p->binfmt->module);
 bad_fork_cleanup_put_domain:
Index: linux-2.6.18-rc5/include/linux/delayacct.h
===================================================================
--- linux-2.6.18-rc5.orig/include/linux/delayacct.h	2006-08-28 11:34:27.000000000 -0400
+++ linux-2.6.18-rc5/include/linux/delayacct.h	2006-08-28 11:34:48.000000000 -0400
@@ -59,10 +59,14 @@ static inline void delayacct_tsk_init(st
 		__delayacct_tsk_init(tsk);
 }

-static inline void delayacct_tsk_exit(struct task_struct *tsk)
+/* Free tsk->delays. Called from bad fork and __put_task_struct
+ * where there's no risk of tsk->delays being accessed elsewhere
+ */
+static inline void delayacct_tsk_free(struct task_struct *tsk)
 {
 	if (tsk->delays)
-		__delayacct_tsk_exit(tsk);
+		kmem_cache_free(delayacct_cache, tsk->delays);
+	tsk->delays = NULL;
 }

 static inline void delayacct_blkio_start(void)
@@ -101,7 +105,7 @@ static inline void delayacct_init(void)
 {}
 static inline void delayacct_tsk_init(struct task_struct *tsk)
 {}
-static inline void delayacct_tsk_exit(struct task_struct *tsk)
+static inline void delayacct_tsk_free(struct task_struct *tsk)
 {}
 static inline void delayacct_blkio_start(void)
 {}
Index: linux-2.6.18-rc5/include/linux/sched.h
===================================================================
--- linux-2.6.18-rc5.orig/include/linux/sched.h	2006-08-28 11:34:27.000000000 -0400
+++ linux-2.6.18-rc5/include/linux/sched.h	2006-08-28 11:34:48.000000000 -0400
@@ -994,7 +994,6 @@ struct task_struct {
 	 */
 	struct pipe_inode_info *splice_pipe;
 #ifdef	CONFIG_TASK_DELAY_ACCT
-	spinlock_t delays_lock;
 	struct task_delay_info *delays;
 #endif
 };
Index: linux-2.6.18-rc5/kernel/exit.c
===================================================================
--- linux-2.6.18-rc5.orig/kernel/exit.c	2006-08-28 11:34:27.000000000 -0400
+++ linux-2.6.18-rc5/kernel/exit.c	2006-08-28 11:34:48.000000000 -0400
@@ -908,7 +908,6 @@ fastcall NORET_TYPE void do_exit(long co
 		audit_free(tsk);
 	taskstats_exit_send(tsk, tidstats, group_dead, mycpu);
 	taskstats_exit_free(tidstats);
-	delayacct_tsk_exit(tsk);

 	exit_mm(tsk);

Index: linux-2.6.18-rc5/kernel/delayacct.c
===================================================================
--- linux-2.6.18-rc5.orig/kernel/delayacct.c	2006-08-28 11:34:27.000000000 -0400
+++ linux-2.6.18-rc5/kernel/delayacct.c	2006-08-28 11:34:48.000000000 -0400
@@ -41,24 +41,11 @@ void delayacct_init(void)

 void __delayacct_tsk_init(struct task_struct *tsk)
 {
-	spin_lock_init(&tsk->delays_lock);
-	/* No need to acquire tsk->delays_lock for allocation here unless
-	   __delayacct_tsk_init called after tsk is attached to tasklist
-	*/
 	tsk->delays = kmem_cache_zalloc(delayacct_cache, SLAB_KERNEL);
 	if (tsk->delays)
 		spin_lock_init(&tsk->delays->lock);
 }

-void __delayacct_tsk_exit(struct task_struct *tsk)
-{
-	struct task_delay_info *delays = tsk->delays;
-	spin_lock(&tsk->delays_lock);
-	tsk->delays = NULL;
-	spin_unlock(&tsk->delays_lock);
-	kmem_cache_free(delayacct_cache, delays);
-}
-
 /*
  * Start accounting for a delay statistic using
  * its starting timestamp (@start)
@@ -118,8 +105,6 @@ int __delayacct_add_tsk(struct taskstats
 	struct timespec ts;
 	unsigned long t1,t2,t3;

-	spin_lock(&tsk->delays_lock);
-
 	/* Though tsk->delays accessed later, early exit avoids
 	 * unnecessary returning of other data
 	 */
@@ -161,7 +146,6 @@ int __delayacct_add_tsk(struct taskstats
 	spin_unlock(&tsk->delays->lock);

 done:
-	spin_unlock(&tsk->delays_lock);
 	return 0;
 }




-
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