Index: kern_sysctl.c =================================================================== --- src/sys/kern/kern_sysctl.c (.../current) (revision 81) +++ src/sys/kern/kern_sysctl.c (.../sysctl_sx) (revision 117) @@ -62,12 +62,21 @@ /* * Locking - this locks the sysctl tree in memory. */ -static struct sx sysctllock; +static struct mtx sysctl_ctx_mtx; +static struct sx sysctl_sx; -#define SYSCTL_LOCK() sx_xlock(&sysctllock) -#define SYSCTL_UNLOCK() sx_xunlock(&sysctllock) -#define SYSCTL_INIT() sx_init(&sysctllock, "sysctl lock") +#define SYSCTL_XLOCK() sx_xlock(&sysctl_sx) +#define SYSCTL_SLOCK() sx_slock(&sysctl_sx) +#define SYSCTL_XUNLOCK() sx_xunlock(&sysctl_sx) +#define SYSCTL_SUNLOCK() sx_sunlock(&sysctl_sx) +#define SYSCTL_INIT() sx_init(&sysctl_sx, "sysctl sx lock") +#define SYSCTL_CTX_LOCK() mtx_lock(&sysctl_ctx_mtx) +#define SYSCTL_CTX_UNLOCK() mtx_unlock(&sysctl_ctx_mtx) +#define SYSCTL_CTX_INIT() mtx_init(&sysctl_ctx_mtx, \ + "sysctl context lock", \ + NULL, MTX_DEF) + static int sysctl_root(SYSCTL_HANDLER_ARGS); struct sysctl_oid_list sysctl__children; /* root list */ @@ -77,6 +86,8 @@ { struct sysctl_oid *oidp; + sx_assert(&sysctl_sx, SX_LOCKED); + SLIST_FOREACH(oidp, list, oid_link) { if (strcmp(oidp->oid_name, name) == 0) { return (oidp); @@ -91,25 +102,39 @@ * Order by number in each list. */ -void +int sysctl_register_oid(struct sysctl_oid *oidp) { struct sysctl_oid_list *parent = oidp->oid_parent; struct sysctl_oid *p; struct sysctl_oid *q; + int error; + int locked; + locked = sx_xlocked(&sysctl_sx); + if (!locked) + SYSCTL_XLOCK(); + /* * First check if another oid with the same name already * exists in the parent's list. */ + + if (parent == NULL) { /* The oid has no parent! */ + error = 1; + goto done; + } + p = sysctl_find_oidname(oidp->oid_name, parent); if (p != NULL) { if ((p->oid_kind & CTLTYPE) == CTLTYPE_NODE) { p->oid_refcnt++; - return; + error = 1; + goto done; } else { printf("can't re-use a leaf (%s)!\n", p->oid_name); - return; + error = 1; + goto done; } } /* @@ -146,19 +171,30 @@ SLIST_INSERT_AFTER(q, oidp, oid_link); else SLIST_INSERT_HEAD(parent, oidp, oid_link); + error = 0; +done: + if (!locked) + SYSCTL_XUNLOCK(); + return (error); } void sysctl_unregister_oid(struct sysctl_oid *oidp) { struct sysctl_oid *p; + struct sysctl_oid_list *parent = oidp->oid_parent; int error; + int locked; + locked = sx_xlocked(&sysctl_sx); + if (!locked) + SYSCTL_XLOCK(); + error = ENOENT; if (oidp->oid_number == OID_AUTO) { error = EINVAL; } else { - SLIST_FOREACH(p, oidp->oid_parent, oid_link) { + SLIST_FOREACH(p, parent, oid_link) { if (p == oidp) { SLIST_REMOVE(oidp->oid_parent, oidp, sysctl_oid, oid_link); @@ -167,7 +203,8 @@ } } } - + if (!locked) + SYSCTL_XUNLOCK(); /* * This can happen when a module fails to register and is * being unloaded afterwards. It should not be a panic() @@ -203,6 +240,8 @@ * XXX This algorithm is a hack. But I don't know any * XXX better solution for now... */ + + SYSCTL_CTX_LOCK(); TAILQ_FOREACH(e, clist, link) { error = sysctl_remove_oid(e->entry, 0, 0); if (error) @@ -221,8 +260,10 @@ sysctl_register_oid(e1->entry); e1 = TAILQ_PREV(e1, sysctl_ctx_list, link); } - if (error) + if (error) { + SYSCTL_CTX_UNLOCK(); return(EBUSY); + } /* Now really delete the entries */ e = TAILQ_FIRST(clist); while (e != NULL) { @@ -234,6 +275,7 @@ free(e, M_SYSCTLOID); e = e1; } + SYSCTL_CTX_UNLOCK(); return (error); } @@ -247,7 +289,9 @@ return(NULL); e = malloc(sizeof(struct sysctl_ctx_entry), M_SYSCTLOID, M_WAITOK); e->entry = oidp; + SYSCTL_CTX_LOCK(); TAILQ_INSERT_HEAD(clist, e, link); + SYSCTL_CTX_UNLOCK(); return (e); } @@ -278,17 +322,21 @@ if (clist == NULL || oidp == NULL) return (EINVAL); + SYSCTL_CTX_LOCK(); e = sysctl_ctx_entry_find(clist, oidp); if (e != NULL) { TAILQ_REMOVE(clist, e, link); free(e, M_SYSCTLOID); + SYSCTL_CTX_UNLOCK(); return (0); - } else + } else { + SYSCTL_CTX_UNLOCK(); return (ENOENT); + } } /* - * Remove dynamically created sysctl trees. + * Really remove dynamically created sysctl trees. * oidp - top of the tree to be removed * del - if 0 - just deregister, otherwise free up entries as well * recurse - if != 0 traverse the subtree to be deleted @@ -298,12 +346,23 @@ { struct sysctl_oid *p; int error; + int locked; if (oidp == NULL) return(EINVAL); + + /* + * Only aquire the exclusive lock if we are really going to delete a + * node, and we don't already hold it. + */ + locked = sx_xlocked(&sysctl_sx); + if (del && (!locked)) + SYSCTL_XLOCK(); + if ((oidp->oid_kind & CTLFLAG_DYN) == 0) { printf("can't remove non-dynamic nodes!\n"); - return (EINVAL); + error = EINVAL; + goto done; } /* * WARNING: normal method to do this should be through @@ -315,23 +374,26 @@ if ((oidp->oid_kind & CTLTYPE) == CTLTYPE_NODE) { if (oidp->oid_refcnt == 1) { SLIST_FOREACH(p, SYSCTL_CHILDREN(oidp), oid_link) { - if (!recurse) - return (ENOTEMPTY); + if (!recurse) { + error = ENOTEMPTY; + goto done; + } error = sysctl_remove_oid(p, del, recurse); if (error) - return (error); + goto done; } if (del) free(SYSCTL_CHILDREN(oidp), M_SYSCTLOID); } } - if (oidp->oid_refcnt > 1 ) { + if (oidp->oid_refcnt > 1) { oidp->oid_refcnt--; } else { if (oidp->oid_refcnt == 0) { printf("Warning: bad oid_refcnt=%u (%s)!\n", oidp->oid_refcnt, oidp->oid_name); - return (EINVAL); + error = EINVAL; + goto done; } sysctl_unregister_oid(oidp); if (del) { @@ -342,7 +404,11 @@ free(oidp, M_SYSCTLOID); } } - return (0); + error = 0; +done: + if (del && (!locked)) + SYSCTL_XUNLOCK(); + return (error); } /* @@ -361,6 +427,8 @@ /* You have to hook up somewhere.. */ if (parent == NULL) return(NULL); + + SYSCTL_XLOCK(); /* Check if the node already exists, otherwise create it */ oidp = sysctl_find_oidname(name, parent); if (oidp != NULL) { @@ -369,9 +437,11 @@ /* Update the context */ if (clist != NULL) sysctl_ctx_entry_add(clist, oidp); + SYSCTL_XUNLOCK(); return (oidp); } else { printf("can't re-use a leaf (%s)!\n", name); + SYSCTL_XUNLOCK(); return (NULL); } } @@ -406,8 +476,20 @@ /* Update the context, if used */ if (clist != NULL) sysctl_ctx_entry_add(clist, oidp); + /* Register this oid */ - sysctl_register_oid(oidp); + if (sysctl_register_oid(oidp)) { + free((void *)(uintptr_t)(const void *)oidp->descr, + M_SYSCTLOID); + free((void *)(uintptr_t)(const void *)oidp->oid_name, + M_SYSCTLOID); + free(oidp->oid_arg1, M_SYSCTLOID); /* Children */ + free(newname, M_SYSCTLOID); + free(oidp, M_SYSCTLOID); + return (NULL); + } + + SYSCTL_XUNLOCK(); return (oidp); } @@ -421,13 +503,17 @@ if (oid->oid_parent == parent) return (0); + SYSCTL_XLOCK(); oidp = sysctl_find_oidname(oid->oid_name, parent); - if (oidp != NULL) + if (oidp != NULL) { + SYSCTL_XUNLOCK(); return (EEXIST); + } sysctl_unregister_oid(oid); oid->oid_parent = parent; oid->oid_number = OID_AUTO; sysctl_register_oid(oid); + SYSCTL_XUNLOCK(); return (0); } @@ -441,11 +527,12 @@ { struct sysctl_oid **oidp; + SYSCTL_CTX_INIT(); SYSCTL_INIT(); SET_FOREACH(oidp, sysctl_set) sysctl_register_oid(*oidp); } -SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_ANY, sysctl_register_all, 0); +SYSINIT(sysctl, SI_SUB_LOCK, SI_ORDER_ANY, sysctl_register_all, 0); /* * "Staff-functions" @@ -513,7 +600,9 @@ error = suser(req->td); if (error) return error; + SYSCTL_SLOCK(); sysctl_sysctl_debug_dump_node(&sysctl__children, 0); + SYSCTL_SUNLOCK(); return ENOENT; } @@ -531,6 +620,7 @@ struct sysctl_oid_list *lsp = &sysctl__children, *lsp2; char buf[10]; + SYSCTL_SLOCK(); while (namelen) { if (!lsp) { snprintf(buf,sizeof(buf),"%d",*name); @@ -553,14 +643,14 @@ error = SYSCTL_OUT(req, ".", 1); if (!error) error = SYSCTL_OUT(req, oid->oid_name, - strlen(oid->oid_name)); + strlen(oid->oid_name)); if (error) return (error); namelen--; name++; - if ((oid->oid_kind & CTLTYPE) != CTLTYPE_NODE) + if ((oid->oid_kind & CTLTYPE) != CTLTYPE_NODE) break; if (oid->oid_handler) @@ -571,7 +661,9 @@ } lsp = lsp2; } - return (SYSCTL_OUT(req, "", 1)); + error = SYSCTL_OUT(req, "", 1); + SYSCTL_SUNLOCK(); + return (error); } SYSCTL_NODE(_sysctl, 1, name, CTLFLAG_RD, sysctl_sysctl_name, ""); @@ -583,23 +675,28 @@ struct sysctl_oid *oidp; *len = level; + SYSCTL_SLOCK(); SLIST_FOREACH(oidp, lsp, oid_link) { *next = oidp->oid_number; *oidpp = oidp; - if (oidp->oid_kind & CTLFLAG_SKIP) continue; - if (!namelen) { - if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) + if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) { + SYSCTL_SUNLOCK(); return 0; - if (oidp->oid_handler) + } + if (oidp->oid_handler) { /* We really should call the handler here...*/ + SYSCTL_SUNLOCK(); return 0; + } lsp = (struct sysctl_oid_list *)oidp->oid_arg1; if (!sysctl_sysctl_next_ls(lsp, 0, 0, next+1, - len, level+1, oidpp)) + len, level+1, oidpp)) { + SYSCTL_SUNLOCK(); return 0; + } goto emptynode; } @@ -607,14 +704,20 @@ continue; if (oidp->oid_number > *name) { - if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) + if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) { + SYSCTL_SUNLOCK(); return 0; - if (oidp->oid_handler) + } + if (oidp->oid_handler) { + SYSCTL_SUNLOCK(); return 0; + } lsp = (struct sysctl_oid_list *)oidp->oid_arg1; if (!sysctl_sysctl_next_ls(lsp, name+1, namelen-1, - next+1, len, level+1, oidpp)) + next+1, len, level+1, oidpp)) { + SYSCTL_SUNLOCK(); return (0); + } goto next; } if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE) @@ -625,13 +728,17 @@ lsp = (struct sysctl_oid_list *)oidp->oid_arg1; if (!sysctl_sysctl_next_ls(lsp, name+1, namelen-1, next+1, - len, level+1, oidpp)) + len, level+1, oidpp)) { + SYSCTL_SUNLOCK(); return (0); + } + next: namelen = 1; emptynode: *len = level; } + SYSCTL_SUNLOCK(); return 1; } @@ -644,7 +751,7 @@ struct sysctl_oid *oid; struct sysctl_oid_list *lsp = &sysctl__children; int newoid[CTL_MAXNAME]; - + i = sysctl_sysctl_next_ls(lsp, name, namelen, newoid, &j, 1, &oid); if (i) return ENOENT; @@ -677,6 +784,7 @@ if (i == '.') *p = '\0'; + SYSCTL_SLOCK(); oidp = SLIST_FIRST(lsp); while (oidp && *len < CTL_MAXNAME) { @@ -690,6 +798,7 @@ if (!i) { if (oidpp) *oidpp = oidp; + SYSCTL_SUNLOCK(); return (0); } @@ -708,6 +817,7 @@ if (i == '.') *p = '\0'; } + SYSCTL_SUNLOCK(); return ENOENT; } @@ -753,16 +863,21 @@ struct sysctl_oid *oid; int error; + SYSCTL_SLOCK(); error = sysctl_find_oid(arg1, arg2, &oid, NULL, req); if (error) - return (error); + goto done; - if (!oid->oid_fmt) - return (ENOENT); + if (!oid->oid_fmt) { + error = ENOENT; + goto done; + } error = SYSCTL_OUT(req, &oid->oid_kind, sizeof(oid->oid_kind)); if (error) - return (error); + goto done; error = SYSCTL_OUT(req, oid->oid_fmt, strlen(oid->oid_fmt) + 1); +done: + SYSCTL_SUNLOCK(); return (error); } @@ -774,14 +889,20 @@ { struct sysctl_oid *oid; int error; - + + SYSCTL_SLOCK(); error = sysctl_find_oid(arg1, arg2, &oid, NULL, req); - if (error) + if (error) { + SYSCTL_SUNLOCK(); return (error); + } - if (!oid->descr) + if (!oid->descr) { + SYSCTL_SUNLOCK(); return (ENOENT); + } error = SYSCTL_OUT(req, oid->descr, strlen(oid->descr) + 1); + SYSCTL_SUNLOCK(); return (error); } @@ -1010,15 +1131,11 @@ req.newfunc = sysctl_new_kernel; req.lock = REQ_LOCKED; - SYSCTL_LOCK(); - error = sysctl_root(0, name, namelen, &req); if (req.lock == REQ_WIRED && req.validlen > 0) vsunlock(req.oldptr, req.validlen); - SYSCTL_UNLOCK(); - if (error && error != ENOMEM) return (error); @@ -1131,14 +1248,21 @@ return (0); } +/* + * If succesful, the returned oid will be already owned. + */ int sysctl_find_oid(int *name, u_int namelen, struct sysctl_oid **noid, int *nindx, struct sysctl_req *req) { struct sysctl_oid *oid; + struct sysctl_oid_list *lsp = &sysctl__children; + struct sysctl_oid_list *parent; int indx; - oid = SLIST_FIRST(&sysctl__children); + sx_assert(&sysctl_sx, SX_LOCKED); + + oid = SLIST_FIRST(lsp); indx = 0; while (oid && indx < CTL_MAXNAME) { if (oid->oid_number == name[indx]) { @@ -1153,8 +1277,8 @@ *nindx = indx; return (0); } - oid = SLIST_FIRST( - (struct sysctl_oid_list *)oid->oid_arg1); + lsp = (struct sysctl_oid_list *)oid->oid_arg1; + oid = SLIST_FIRST(lsp); } else if (indx == namelen) { *noid = oid; if (nindx != NULL) @@ -1164,7 +1288,10 @@ return (ENOTDIR); } } else { + parent = oid->oid_parent; oid = SLIST_NEXT(oid, oid_link); + if (!oid) + return (ENOENT); } } return (ENOENT); @@ -1180,10 +1307,11 @@ { struct sysctl_oid *oid; int error, indx, lvl; - + + SYSCTL_SLOCK(); error = sysctl_find_oid(arg1, arg2, &oid, &indx, req); if (error) - return (error); + goto done; if ((oid->oid_kind & CTLTYPE) == CTLTYPE_NODE) { /* @@ -1191,13 +1319,17 @@ * no handler. Inform the user that it's a node. * The indx may or may not be the same as namelen. */ - if (oid->oid_handler == NULL) - return (EISDIR); + if (oid->oid_handler == NULL) { + error = EISDIR; + goto done; + } } /* Is this sysctl writable? */ - if (req->newptr && !(oid->oid_kind & CTLFLAG_WR)) - return (EPERM); + if (req->newptr && !(oid->oid_kind & CTLFLAG_WR)) { + error = EPERM; + goto done; + } KASSERT(req->td != NULL, ("sysctl_root(): req->td == NULL")); @@ -1206,7 +1338,7 @@ lvl = (oid->oid_kind & CTLMASK_SECURE) >> CTLSHIFT_SECURE; error = securelevel_gt(req->td->td_ucred, lvl); if (error) - return (error); + goto done; } /* Is this sysctl writable by only privileged users? */ @@ -1219,11 +1351,13 @@ flags = 0; error = suser_cred(req->td->td_ucred, flags); if (error) - return (error); + goto done; } - if (!oid->oid_handler) - return EINVAL; + if (!oid->oid_handler) { + error = EINVAL; + goto done; + } if ((oid->oid_kind & CTLTYPE) == CTLTYPE_NODE) { arg1 = (int *)arg1 + indx; @@ -1236,10 +1370,15 @@ error = mac_check_system_sysctl(req->td->td_ucred, oid, arg1, arg2, req); if (error != 0) - return (error); + goto done; + #endif + mtx_lock(&Giant); error = oid->oid_handler(oid, arg1, arg2, req); + mtx_unlock(&Giant); +done: + SYSCTL_SUNLOCK(); return (error); } @@ -1270,8 +1409,6 @@ if (error) return (error); - mtx_lock(&Giant); - error = userland_sysctl(td, name, uap->namelen, uap->old, uap->oldlenp, 0, uap->new, uap->newlen, &j, 0); @@ -1283,7 +1420,6 @@ error = i; } done2: - mtx_unlock(&Giant); return (error); } @@ -1332,8 +1468,6 @@ req.newfunc = sysctl_new_user; req.lock = REQ_LOCKED; - SYSCTL_LOCK(); - do { req.oldidx = 0; req.newidx = 0; @@ -1343,8 +1477,6 @@ if (req.lock == REQ_WIRED && req.validlen > 0) vsunlock(req.oldptr, req.validlen); - SYSCTL_UNLOCK(); - if (error && error != ENOMEM) return (error); Index: sysctl.h =================================================================== --- src/sys/sys/sysctl.h (.../current) (revision 81) +++ src/sys/sys/sysctl.h (.../sysctl_sx) (revision 117) @@ -176,7 +176,7 @@ /* * These functions are used to add/remove an oid from the mib. */ -void sysctl_register_oid(struct sysctl_oid *oidp); +int sysctl_register_oid(struct sysctl_oid *oidp); void sysctl_unregister_oid(struct sysctl_oid *oidp); /* Declare a static oid to allow child oids to be added to it. */