DF-2753 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 | fix.diff for DF-2753: initialize p2->p_pgrp/p_ucred (and the rest of the startcopy region) BEFORE proc_add_allproc() publishes the SIDL child, and filter SIDL in pfind()/allproc_scan(). Authored after guest verification (panic reproduced in p_trespass+0x3f, fault VA 0xa0). --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -476,34 +476,20 @@ M_SUBPROC, M_WAITOK | M_ZERO); /* - * Setup linkage for kernel based threading XXX lwp. Also add the - * process to the allproclist. + * Initialize the section which is copied verbatim from the parent + * and install the child's credentials BEFORE the process becomes + * addressable. * - * The process structure is addressable after this point. - */ - if (flags & RFTHREAD) { - p2->p_peers = p1->p_peers; - p1->p_peers = p2; - p2->p_leader = p1->p_leader; - } else { - p2->p_leader = p2; - } - proc_add_allproc(p2); - - /* - * Initialize the section which is copied verbatim from the parent. + * The process structure becomes addressable once proc_add_allproc() + * executes, and pfind()/allproc_scan() do not filter SIDL processes. + * Consumers such as kill(-1) -> p_trespass() dereference p->p_ucred, + * and getsid() dereferences p->p_pgrp (via p_session), without + * holding p_token. These fields MUST be valid before publication + * or a concurrent lookup can take a NULL-pointer fault. */ bcopy(&p1->p_startcopy, &p2->p_startcopy, ((caddr_t)&p2->p_endcopy - (caddr_t)&p2->p_startcopy)); - /* - * Duplicate sub-structures as needed. Increase reference counts - * on shared objects. - * - * NOTE: because we are now on the allproc list it is possible for - * other consumers to gain temporary references to p2 - * (p2->p_lock can change). - */ if (p1->p_flags & P_PROFIL) startprofclock(p2); p2->p_ucred = crhold(lp1->lwp_thread->td_ucred); @@ -514,6 +500,23 @@ if (p2->p_args) refcount_acquire(&p2->p_args->ar_ref); + /* + * Setup linkage for kernel based threading XXX lwp. Also add the + * process to the allproclist. + * + * The process structure is addressable after this point. Other + * consumers can gain temporary references to p2 from here on + * (p2->p_lock can change). + */ + if (flags & RFTHREAD) { + p2->p_peers = p1->p_peers; + p1->p_peers = p2; + p2->p_leader = p1->p_leader; + } else { + p2->p_leader = p2; + } + proc_add_allproc(p2); + p2->p_usched = p1->p_usched; /* XXX: verify copy of the secondary iosched stuff */ dsched_enter_proc(p2); --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -521,7 +521,7 @@ lwkt_gettoken_shared(&prg->proc_token); LIST_FOREACH(p, &prg->allproc, p_list) { - if (p->p_stat == SZOMB) + if (p->p_stat == SIDL || p->p_stat == SZOMB) continue; if (p->p_pid == pid) { PHOLD(p); @@ -1427,7 +1427,7 @@ continue; lwkt_gettoken(&prg->proc_token); LIST_FOREACH(p, &prg->allproc, p_list) { - if (p->p_stat == SZOMB) + if (p->p_stat == SIDL || p->p_stat == SZOMB) continue; PHOLD(p); r = callback(p, data); @@ -1481,7 +1481,7 @@ continue; lwkt_gettoken(&prg->proc_token); LIST_FOREACH(p, &prg->allproc, p_list) { - if (p->p_stat == SZOMB) + if (p->p_stat == SIDL || p->p_stat == SZOMB) continue; PHOLD(p); lwkt_gettoken(&p->p_token); |