DF-2800 / 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 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 | --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -210,6 +210,31 @@ error = assign_prison_id(pr); if (error) { + if (pr->pr_root.ncp) + cache_drop(&pr->pr_root); + varsymset_clean(&pr->pr_varsymset); + nlookup_done(&nd); + return (error); + } + + /* + * Create the per-prison sysctl tree BEFORE publishing the prison + * on allprison and WITHOUT holding jail_lock. + * + * prison_sysctl_create() -> SYSCTL_ADD_* -> sysctl_add_oid() takes + * the all-CPU sysctl xlock (_sysctl_xlock), while the + * sysctl_jail_list() handler takes jail_lock while holding a + * per-CPU shared gd_sysctllock. Holding jail_lock across the + * sysctl tree creation/destruction is a guaranteed AB-BA deadlock + * that wedges the whole kernel ( DF-2799 ). Creating the tree + * before publication also guarantees no sysctl node for a given + * prison id can be observed or shared while its prison is being + * torn down ( DF-2802 ). + */ + error = prison_sysctl_create(pr); + if (error) { + if (pr->pr_root.ncp) + cache_drop(&pr->pr_root); varsymset_clean(&pr->pr_varsymset); nlookup_done(&nd); return (error); @@ -220,25 +245,21 @@ ++prisoncount; lockmgr(&jail_lock, LK_RELEASE); - error = prison_sysctl_create(pr); - if (error) - goto out; - error = kern_jail_attach(pr->pr_id); if (error) - goto out2; + goto out; nlookup_done(&nd); return 0; -out2: - prison_sysctl_done(pr); - out: lockmgr(&jail_lock, LK_EXCLUSIVE); LIST_REMOVE(pr, pr_list); --prisoncount; lockmgr(&jail_lock, LK_RELEASE); + prison_sysctl_done(pr); + if (pr->pr_root.ncp) + cache_drop(&pr->pr_root); varsymset_clean(&pr->pr_varsymset); nlookup_done(&nd); return (error); @@ -325,17 +346,25 @@ error = copyinstr(j.hostname, &pr->pr_host, sizeof(pr->pr_host), 0); if (error) - goto out; + goto out_locked; /* Use default capabilities as a template */ pr->pr_caps = prison_default_caps; + /* + * kern_jail() manages the jail_lock itself. The lock is NOT held + * across kern_jail() because prison_sysctl_create() / + * prison_sysctl_done() take the all-CPU sysctl xlock, which + * deadlocks against sysctl_jail_list() readers holding a per-CPU + * shared gd_sysctllock and blocking on jail_lock ( DF-2799 ). + */ + lockmgr(&jail_lock, LK_RELEASE); + error = kern_jail(pr, &j); if (error) goto out; sysmsg->sysmsg_result = pr->pr_id; - lockmgr(&jail_lock, LK_RELEASE); return (0); @@ -346,6 +375,19 @@ SLIST_REMOVE_HEAD(&pr->pr_ips, entries); kfree(jip, M_PRISON); } + if (pr->pr_root.ncp) + cache_drop(&pr->pr_root); /* DF-2801 */ + kfree(pr, M_PRISON); + + return (error); + +out_locked: + /* Delete all ips */ + while (!SLIST_EMPTY(&pr->pr_ips)) { + jip = SLIST_FIRST(&pr->pr_ips); + SLIST_REMOVE_HEAD(&pr->pr_ips, entries); + kfree(jip, M_PRISON); + } lockmgr(&jail_lock, LK_RELEASE); kfree(pr, M_PRISON); @@ -889,9 +931,15 @@ case SYSCAP_NODEBUG_UNPRIV: case SYSCAP_NOSCHED: case SYSCAP_NOSCHED_CPUSET: - case SYSCAP_NOSETTIME: return (0); + case SYSCAP_NOSETTIME: + /* + * The system clock is host-global state; jailed root + * must not be able to step or drift it ( DF-2800 ). + */ + return (EPERM); + case SYSCAP_NOEXEC_SUID: /* group 3 allowed */ case SYSCAP_NOEXEC_SGID: return (0); @@ -989,9 +1037,17 @@ int prison_sysctl_create(struct prison *pr) { - char id_str[7]; + char id_str[12]; - ksnprintf(id_str, 6, "%d", pr->pr_id); + /* + * Full decimal representation of any pr_id < JAIL_MAX (999999) + * plus NUL. The old 5-character truncation made every prison + * with id >= 100000 share the sysctl node of the first prison + * in its 1000-id band ("10000", "11000", ...), silently + * dropping all per-jail capability controls and corrupting the + * shared node's reference accounting at teardown ( DF-2802 ). + */ + ksnprintf(id_str, sizeof(id_str), "%d", pr->pr_id); pr->pr_sysctl_ctx = (struct sysctl_ctx_list *) kmalloc( sizeof(struct sysctl_ctx_list), M_PRISON, M_WAITOK | M_ZERO); --- a/sys/kern/kern_caps.c +++ b/sys/kern/kern_caps.c @@ -311,6 +311,7 @@ caps_priv_check(struct ucred *cred, int cap) { int res; + int ocap = cap; if (cred == NULL) { if (cap & __SYSCAP_NULLCRED) @@ -337,7 +338,13 @@ } if (res & __SYSCAP_SELF) return EPERM; - return (prison_priv_check(cred, cap)); + /* + * prison_priv_check() must see the ORIGINAL capability so its + * per-leaf jail policy applies; passing only the rewritten group + * meta value blanket-allows whole capability groups in jails + * ( DF-2800 ). + */ + return (prison_priv_check(cred, ocap)); } int --- a/sys/kern/kern_sysctl.c +++ b/sys/kern/kern_sysctl.c @@ -373,7 +373,15 @@ } } if (oidp->oid_refcnt > 1 ) { - oidp->oid_refcnt--; + /* + * A dry run (del == 0) must not consume a reference: + * sysctl_ctx_free() dry-runs every context entry first, + * so decrementing here frees shared dynamic nodes (e.g. + * prison trees) while other contexts still reference + * them ( DF-2802 ). + */ + if (del) + oidp->oid_refcnt--; } else { if (oidp->oid_refcnt == 0) { kprintf("Warning: bad oid_refcnt=%u (%s)!\n", |