用户工具

站点工具


hacking:f:fork_race

FreeBSD 6.2 fork/[sg]etpriority竞态条件的修正

发现

在线上系统运行过程中,quakelee 报告在使用renice -u www +20时系统会出现特征为 Kernel Trap 12 的崩溃。

诊断

经过调试发现,内核中的 getpriority/setpriority 的 PRIO_USER 需要访问进程的 p_ucred 指针指向的信任状数据,这个数据由进程结构的 proc mutex 保护。然而,某些情况下,p_ucred可能是NULL,从而导致空指针引用并引致 Kernel Trap 12,具体说来,在 fork() 的过程中,尽管所有操作均做了正确的上锁,但存在这样的情形,即,拿到 proc mtx 时 p_ucred 为 NULL,这种状态出现在进程刚刚被复制(PRS_NEW),但还没有完成复制的阶段。

修正

查看 sys/sys/proc.h 中 struct proc 的定义,其中关于 p_ucred 的定义为:

struct proc {
[...]
        struct ucred    *p_ucred;       /* (c) Process owner's identity. */

因此,需要对新进程上 proc_mtx 锁。由于遍历 allproc 表需要首先获得 allproc_lock 的 sx_xlock,而遍历该表是访问 proc 结构的先决条件,故可在 allproc_lock 之前获得 proc lock,然后再释放 allproc_lock。

具体的调整包括:

  • 对父进程、子进程分别上锁。其中,父进程的锁在完成 startcopy 到 endcopy 小节的复制之后立即释放。(此操作原先是在接近完成时进行)
  • 在对子进程的 startzero 到 endzero 小节的清零完成、并复制了 ucred 之后,释放子进程的 proc lock。
  • kern_resource.c 在遇到 PRS_NEW 进程时,直接跳过而不对进程上 proc lock。这样做的目的是避免子进程正在修改的过程中(此时,fork所在的线程拥有子进程的proc lock)发生撞锁现象。

最终改动如下:

Index: kern_fork.c
===================================================================
--- kern_fork.c	(revision 167006)
+++ kern_fork.c	(revision 167007)
@@ -423,8 +423,22 @@
 	AUDIT_ARG(pid, p2->p_pid);
 	LIST_INSERT_HEAD(&allproc, p2, p_list);
 	LIST_INSERT_HEAD(PIDHASH(p2->p_pid), p2, p_hash);
+
+	PROC_LOCK(p2);
+	PROC_LOCK(p1);
+
 	sx_xunlock(&allproc_lock);
 
+	bcopy(&p1->p_startcopy, &p2->p_startcopy,
+	    __rangeof(struct proc, p_startcopy, p_endcopy));
+	PROC_UNLOCK(p1);
+
+	bzero(&p2->p_startzero,
+	    __rangeof(struct proc, p_startzero, p_endzero));
+
+	p2->p_ucred = crhold(td->td_ucred);
+	PROC_UNLOCK(p2);
+
 	/*
 	 * Malloc things while we don't hold any locks.
 	 */
@@ -482,13 +496,9 @@
 	PROC_LOCK(p2);
 	PROC_LOCK(p1);
 
-	bzero(&p2->p_startzero,
-	    __rangeof(struct proc, p_startzero, p_endzero));
 	bzero(&td2->td_startzero,
 	    __rangeof(struct thread, td_startzero, td_endzero));
 
-	bcopy(&p1->p_startcopy, &p2->p_startcopy,
-	    __rangeof(struct proc, p_startcopy, p_endcopy));
 	bcopy(&td->td_startcopy, &td2->td_startcopy,
 	    __rangeof(struct thread, td_startcopy, td_endcopy));
 
@@ -511,7 +521,6 @@
 	sched_fork(td, td2);
 
 	mtx_unlock_spin(&sched_lock);
-	p2->p_ucred = crhold(td->td_ucred);
 	td2->td_ucred = crhold(p2->p_ucred);
 #ifdef AUDIT
 	audit_proc_fork(p1, p2);
Index: kern_resource.c
===================================================================
--- kern_resource.c	(revision 167006)
+++ kern_resource.c	(revision 167007)
@@ -143,6 +143,9 @@
 			uap->who = td->td_ucred->cr_uid;
 		sx_slock(&allproc_lock);
 		FOREACH_PROC_IN_SYSTEM(p) {
+			/* Do not bother to check PRS_NEW processes */
+			if (p->p_state == PRS_NEW)
+				continue;
 			PROC_LOCK(p);
 			if (!p_cansee(td, p) &&
 			    p->p_ucred->cr_uid == uap->who) {
/data/vhosts/wiki-data/pages/hacking/f/fork_race.txt · 最后更改: 2010/09/04 08:12 由 delphij