Do not hold the sysctl lock across a call to the handler. This fixes a
general LOR issue where the sysctl lock had no good place in the hierarchy. One specific instance is #284 on http://sources.zabbadoz.net/freebsd/lor.html . Reviewed by: jhb MFC after: 1 month X-MFC-note: split oid_refcnt field for oid_running to preserve KBI
This commit is contained in:
Matthew D Fleming
parent
d0bb6f258b
commit
ccecef29d1
@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
|
||||
#include "opt_ktrace.h"
|
||||
|
||||
#include <sys/param.h>
|
||||
#include <sys/fail.h>
|
||||
#include <sys/systm.h>
|
||||
#include <sys/kernel.h>
|
||||
#include <sys/sysctl.h>
|
||||
@ -86,13 +87,12 @@ static MALLOC_DEFINE(M_SYSCTLTMP, "sysctltmp", "sysctl temp output buffer");
|
||||
static struct sx sysctllock;
|
||||
static struct sx sysctlmemlock;
|
||||
|
||||
#define SYSCTL_SLOCK() sx_slock(&sysctllock)
|
||||
#define SYSCTL_SUNLOCK() sx_sunlock(&sysctllock)
|
||||
#define SYSCTL_XLOCK() sx_xlock(&sysctllock)
|
||||
#define SYSCTL_XUNLOCK() sx_xunlock(&sysctllock)
|
||||
#define SYSCTL_ASSERT_XLOCKED() sx_assert(&sysctllock, SA_XLOCKED)
|
||||
#define SYSCTL_ASSERT_LOCKED() sx_assert(&sysctllock, SA_LOCKED)
|
||||
#define SYSCTL_INIT() sx_init(&sysctllock, "sysctl lock")
|
||||
#define SYSCTL_SLEEP(ch, wmesg, timo) \
|
||||
sx_sleep(ch, &sysctllock, 0, wmesg, timo)
|
||||
|
||||
static int sysctl_root(SYSCTL_HANDLER_ARGS);
|
||||
|
||||
@ -106,7 +106,7 @@ sysctl_find_oidname(const char *name, struct sysctl_oid_list *list)
|
||||
{
|
||||
struct sysctl_oid *oidp;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
SLIST_FOREACH(oidp, list, oid_link) {
|
||||
if (strcmp(oidp->oid_name, name) == 0) {
|
||||
return (oidp);
|
||||
@ -313,7 +313,7 @@ sysctl_ctx_entry_find(struct sysctl_ctx_list *clist, struct sysctl_oid *oidp)
|
||||
{
|
||||
struct sysctl_ctx_entry *e;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
if (clist == NULL || oidp == NULL)
|
||||
return(NULL);
|
||||
TAILQ_FOREACH(e, clist, link) {
|
||||
@ -409,6 +409,16 @@ sysctl_remove_oid_locked(struct sysctl_oid *oidp, int del, int recurse)
|
||||
}
|
||||
sysctl_unregister_oid(oidp);
|
||||
if (del) {
|
||||
/*
|
||||
* Wait for all threads running the handler to drain.
|
||||
* This preserves the previous behavior when the
|
||||
* sysctl lock was held across a handler invocation,
|
||||
* and is necessary for module unload correctness.
|
||||
*/
|
||||
while (oidp->oid_running > 0) {
|
||||
oidp->oid_kind |= CTLFLAG_DYING;
|
||||
SYSCTL_SLEEP(&oidp->oid_running, "oidrm", 0);
|
||||
}
|
||||
if (oidp->oid_descr)
|
||||
free((void *)(uintptr_t)(const void *)oidp->oid_descr, M_SYSCTLOID);
|
||||
free((void *)(uintptr_t)(const void *)oidp->oid_name,
|
||||
@ -581,7 +591,7 @@ sysctl_sysctl_debug_dump_node(struct sysctl_oid_list *l, int i)
|
||||
int k;
|
||||
struct sysctl_oid *oidp;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
SLIST_FOREACH(oidp, l, oid_link) {
|
||||
|
||||
for (k=0; k<i; k++)
|
||||
@ -622,7 +632,9 @@ sysctl_sysctl_debug(SYSCTL_HANDLER_ARGS)
|
||||
error = priv_check(req->td, PRIV_SYSCTL_DEBUG);
|
||||
if (error)
|
||||
return (error);
|
||||
SYSCTL_XLOCK();
|
||||
sysctl_sysctl_debug_dump_node(&sysctl__children, 0);
|
||||
SYSCTL_XUNLOCK();
|
||||
return (ENOENT);
|
||||
}
|
||||
|
||||
@ -640,7 +652,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
|
||||
struct sysctl_oid_list *lsp = &sysctl__children, *lsp2;
|
||||
char buf[10];
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_XLOCK();
|
||||
while (namelen) {
|
||||
if (!lsp) {
|
||||
snprintf(buf,sizeof(buf),"%d",*name);
|
||||
@ -649,7 +661,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
|
||||
if (!error)
|
||||
error = SYSCTL_OUT(req, buf, strlen(buf));
|
||||
if (error)
|
||||
return (error);
|
||||
goto out;
|
||||
namelen--;
|
||||
name++;
|
||||
continue;
|
||||
@ -665,7 +677,7 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
|
||||
error = SYSCTL_OUT(req, oid->oid_name,
|
||||
strlen(oid->oid_name));
|
||||
if (error)
|
||||
return (error);
|
||||
goto out;
|
||||
|
||||
namelen--;
|
||||
name++;
|
||||
@ -681,7 +693,10 @@ sysctl_sysctl_name(SYSCTL_HANDLER_ARGS)
|
||||
}
|
||||
lsp = lsp2;
|
||||
}
|
||||
return (SYSCTL_OUT(req, "", 1));
|
||||
error = SYSCTL_OUT(req, "", 1);
|
||||
out:
|
||||
SYSCTL_XUNLOCK();
|
||||
return (error);
|
||||
}
|
||||
|
||||
static SYSCTL_NODE(_sysctl, 1, name, CTLFLAG_RD, sysctl_sysctl_name, "");
|
||||
@ -692,7 +707,7 @@ sysctl_sysctl_next_ls(struct sysctl_oid_list *lsp, int *name, u_int namelen,
|
||||
{
|
||||
struct sysctl_oid *oidp;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
*len = level;
|
||||
SLIST_FOREACH(oidp, lsp, oid_link) {
|
||||
*next = oidp->oid_number;
|
||||
@ -756,7 +771,9 @@ sysctl_sysctl_next(SYSCTL_HANDLER_ARGS)
|
||||
struct sysctl_oid_list *lsp = &sysctl__children;
|
||||
int newoid[CTL_MAXNAME];
|
||||
|
||||
SYSCTL_XLOCK();
|
||||
i = sysctl_sysctl_next_ls(lsp, name, namelen, newoid, &j, 1, &oid);
|
||||
SYSCTL_XUNLOCK();
|
||||
if (i)
|
||||
return (ENOENT);
|
||||
error = SYSCTL_OUT(req, newoid, j * sizeof (int));
|
||||
@ -773,7 +790,7 @@ name2oid(char *name, int *oid, int *len, struct sysctl_oid **oidpp)
|
||||
struct sysctl_oid_list *lsp = &sysctl__children;
|
||||
char *p;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
|
||||
if (!*name)
|
||||
return (ENOENT);
|
||||
@ -831,8 +848,6 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_ARGS)
|
||||
int error, oid[CTL_MAXNAME], len;
|
||||
struct sysctl_oid *op = 0;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
|
||||
if (!req->newlen)
|
||||
return (ENOENT);
|
||||
if (req->newlen >= MAXPATHLEN) /* XXX arbitrary, undocumented */
|
||||
@ -848,7 +863,9 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_ARGS)
|
||||
|
||||
p [req->newlen] = '\0';
|
||||
|
||||
SYSCTL_XLOCK();
|
||||
error = name2oid(p, oid, &len, &op);
|
||||
SYSCTL_XUNLOCK();
|
||||
|
||||
free(p, M_SYSCTL);
|
||||
|
||||
@ -868,16 +885,21 @@ sysctl_sysctl_oidfmt(SYSCTL_HANDLER_ARGS)
|
||||
struct sysctl_oid *oid;
|
||||
int error;
|
||||
|
||||
SYSCTL_XLOCK();
|
||||
error = sysctl_find_oid(arg1, arg2, &oid, NULL, req);
|
||||
if (error)
|
||||
return (error);
|
||||
goto out;
|
||||
|
||||
if (!oid->oid_fmt)
|
||||
return (ENOENT);
|
||||
if (oid->oid_fmt == NULL) {
|
||||
error = ENOENT;
|
||||
goto out;
|
||||
}
|
||||
error = SYSCTL_OUT(req, &oid->oid_kind, sizeof(oid->oid_kind));
|
||||
if (error)
|
||||
return (error);
|
||||
goto out;
|
||||
error = SYSCTL_OUT(req, oid->oid_fmt, strlen(oid->oid_fmt) + 1);
|
||||
out:
|
||||
SYSCTL_XUNLOCK();
|
||||
return (error);
|
||||
}
|
||||
|
||||
@ -891,13 +913,18 @@ sysctl_sysctl_oiddescr(SYSCTL_HANDLER_ARGS)
|
||||
struct sysctl_oid *oid;
|
||||
int error;
|
||||
|
||||
SYSCTL_XLOCK();
|
||||
error = sysctl_find_oid(arg1, arg2, &oid, NULL, req);
|
||||
if (error)
|
||||
return (error);
|
||||
goto out;
|
||||
|
||||
if (!oid->oid_descr)
|
||||
return (ENOENT);
|
||||
if (oid->oid_descr == NULL) {
|
||||
error = ENOENT;
|
||||
goto out;
|
||||
}
|
||||
error = SYSCTL_OUT(req, oid->oid_descr, strlen(oid->oid_descr) + 1);
|
||||
out:
|
||||
SYSCTL_XUNLOCK();
|
||||
return (error);
|
||||
}
|
||||
|
||||
@ -1177,9 +1204,9 @@ kernel_sysctl(struct thread *td, int *name, u_int namelen, void *old,
|
||||
req.newfunc = sysctl_new_kernel;
|
||||
req.lock = REQ_LOCKED;
|
||||
|
||||
SYSCTL_SLOCK();
|
||||
SYSCTL_XLOCK();
|
||||
error = sysctl_root(0, name, namelen, &req);
|
||||
SYSCTL_SUNLOCK();
|
||||
SYSCTL_XUNLOCK();
|
||||
|
||||
if (req.lock == REQ_WIRED && req.validlen > 0)
|
||||
vsunlock(req.oldptr, req.validlen);
|
||||
@ -1307,7 +1334,7 @@ sysctl_find_oid(int *name, u_int namelen, struct sysctl_oid **noid,
|
||||
struct sysctl_oid *oid;
|
||||
int indx;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
lsp = &sysctl__children;
|
||||
indx = 0;
|
||||
while (indx < CTL_MAXNAME) {
|
||||
@ -1326,6 +1353,8 @@ sysctl_find_oid(int *name, u_int namelen, struct sysctl_oid **noid,
|
||||
*noid = oid;
|
||||
if (nindx != NULL)
|
||||
*nindx = indx;
|
||||
KASSERT((oid->oid_kind & CTLFLAG_DYING) == 0,
|
||||
("%s found DYING node %p", __func__, oid));
|
||||
return (0);
|
||||
}
|
||||
lsp = SYSCTL_CHILDREN(oid);
|
||||
@ -1333,6 +1362,8 @@ sysctl_find_oid(int *name, u_int namelen, struct sysctl_oid **noid,
|
||||
*noid = oid;
|
||||
if (nindx != NULL)
|
||||
*nindx = indx;
|
||||
KASSERT((oid->oid_kind & CTLFLAG_DYING) == 0,
|
||||
("%s found DYING node %p", __func__, oid));
|
||||
return (0);
|
||||
} else {
|
||||
return (ENOTDIR);
|
||||
@ -1352,7 +1383,7 @@ sysctl_root(SYSCTL_HANDLER_ARGS)
|
||||
struct sysctl_oid *oid;
|
||||
int error, indx, lvl;
|
||||
|
||||
SYSCTL_ASSERT_LOCKED();
|
||||
SYSCTL_ASSERT_XLOCKED();
|
||||
|
||||
error = sysctl_find_oid(arg1, arg2, &oid, &indx, req);
|
||||
if (error)
|
||||
@ -1416,12 +1447,21 @@ sysctl_root(SYSCTL_HANDLER_ARGS)
|
||||
if (error != 0)
|
||||
return (error);
|
||||
#endif
|
||||
oid->oid_running++;
|
||||
SYSCTL_XUNLOCK();
|
||||
|
||||
if (!(oid->oid_kind & CTLFLAG_MPSAFE))
|
||||
mtx_lock(&Giant);
|
||||
error = oid->oid_handler(oid, arg1, arg2, req);
|
||||
if (!(oid->oid_kind & CTLFLAG_MPSAFE))
|
||||
mtx_unlock(&Giant);
|
||||
|
||||
KFAIL_POINT_ERROR(_debug_fail_point, sysctl_running, error);
|
||||
|
||||
SYSCTL_XLOCK();
|
||||
oid->oid_running--;
|
||||
if (oid->oid_running == 0 && (oid->oid_kind & CTLFLAG_DYING) != 0)
|
||||
wakeup(&oid->oid_running);
|
||||
return (error);
|
||||
}
|
||||
|
||||
@ -1521,9 +1561,9 @@ userland_sysctl(struct thread *td, int *name, u_int namelen, void *old,
|
||||
for (;;) {
|
||||
req.oldidx = 0;
|
||||
req.newidx = 0;
|
||||
SYSCTL_SLOCK();
|
||||
SYSCTL_XLOCK();
|
||||
error = sysctl_root(0, name, namelen, &req);
|
||||
SYSCTL_SUNLOCK();
|
||||
SYSCTL_XUNLOCK();
|
||||
if (error != EAGAIN)
|
||||
break;
|
||||
uio_yield();
|
||||
|
||||
@ -87,6 +87,7 @@ struct ctlname {
|
||||
#define CTLFLAG_MPSAFE 0x00040000 /* Handler is MP safe */
|
||||
#define CTLFLAG_VNET 0x00020000 /* Prisons with vnet can fiddle */
|
||||
#define CTLFLAG_RDTUN (CTLFLAG_RD|CTLFLAG_TUN)
|
||||
#define CTLFLAG_DYING 0x00010000 /* oid is being removed */
|
||||
|
||||
/*
|
||||
* Secure level. Note that CTLFLAG_SECURE == CTLFLAG_SECURE1.
|
||||
@ -163,6 +164,7 @@ struct sysctl_oid {
|
||||
int (*oid_handler)(SYSCTL_HANDLER_ARGS);
|
||||
const char *oid_fmt;
|
||||
int oid_refcnt;
|
||||
u_int oid_running;
|
||||
const char *oid_descr;
|
||||
};
|
||||
|
||||
@ -222,7 +224,7 @@ TAILQ_HEAD(sysctl_ctx_list, sysctl_ctx_entry);
|
||||
#define SYSCTL_OID(parent, nbr, name, kind, a1, a2, handler, fmt, descr) \
|
||||
static struct sysctl_oid sysctl__##parent##_##name = { \
|
||||
&sysctl_##parent##_children, { NULL }, nbr, kind, \
|
||||
a1, a2, #name, handler, fmt, 0, __DESCR(descr) }; \
|
||||
a1, a2, #name, handler, fmt, 0, 0, __DESCR(descr) }; \
|
||||
DATA_SET(sysctl_set, sysctl__##parent##_##name)
|
||||
|
||||
#define SYSCTL_ADD_OID(ctx, parent, nbr, name, kind, a1, a2, handler, fmt, descr) \
|
||||
|
||||
Loading…
Reference in New Issue
Block a user