Changeset View
Standalone View
sys/kern/kern_sysctl.c
Show First 20 Lines • Show All 90 Lines • ▼ Show 20 Lines | |||||
* be held, so the sysctl_wlock() and sysctl_wunlock() routines are | * be held, so the sysctl_wlock() and sysctl_wunlock() routines are | ||||
* provided for the few places in the kernel which need to use that | * provided for the few places in the kernel which need to use that | ||||
* API rather than using the dynamic API. Use of the dynamic API is | * API rather than using the dynamic API. Use of the dynamic API is | ||||
* strongly encouraged for most code. | * strongly encouraged for most code. | ||||
* | * | ||||
* The sysctlmemlock is used to limit the amount of user memory wired for | * The sysctlmemlock is used to limit the amount of user memory wired for | ||||
* sysctl requests. This is implemented by serializing any userland | * sysctl requests. This is implemented by serializing any userland | ||||
* sysctl requests larger than a single page via an exclusive lock. | * sysctl requests larger than a single page via an exclusive lock. | ||||
* | |||||
* The sysctlstringlock is used to protect concurrent access to writable | |||||
* string nodes in sysctl_handle_string(). | |||||
*/ | */ | ||||
static struct rmlock sysctllock; | static struct rmlock sysctllock; | ||||
static struct sx __exclusive_cache_line sysctlmemlock; | static struct sx __exclusive_cache_line sysctlmemlock; | ||||
static struct sx sysctlstringlock; | |||||
#define SYSCTL_WLOCK() rm_wlock(&sysctllock) | #define SYSCTL_WLOCK() rm_wlock(&sysctllock) | ||||
#define SYSCTL_WUNLOCK() rm_wunlock(&sysctllock) | #define SYSCTL_WUNLOCK() rm_wunlock(&sysctllock) | ||||
#define SYSCTL_RLOCK(tracker) rm_rlock(&sysctllock, (tracker)) | #define SYSCTL_RLOCK(tracker) rm_rlock(&sysctllock, (tracker)) | ||||
#define SYSCTL_RUNLOCK(tracker) rm_runlock(&sysctllock, (tracker)) | #define SYSCTL_RUNLOCK(tracker) rm_runlock(&sysctllock, (tracker)) | ||||
#define SYSCTL_WLOCKED() rm_wowned(&sysctllock) | #define SYSCTL_WLOCKED() rm_wowned(&sysctllock) | ||||
#define SYSCTL_ASSERT_LOCKED() rm_assert(&sysctllock, RA_LOCKED) | #define SYSCTL_ASSERT_LOCKED() rm_assert(&sysctllock, RA_LOCKED) | ||||
#define SYSCTL_ASSERT_WLOCKED() rm_assert(&sysctllock, RA_WLOCKED) | #define SYSCTL_ASSERT_WLOCKED() rm_assert(&sysctllock, RA_WLOCKED) | ||||
▲ Show 20 Lines • Show All 55 Lines • ▼ Show 20 Lines | sysctl_root_handler_locked(struct sysctl_oid *oid, void *arg1, intmax_t arg2, | ||||
if (oid->oid_kind & CTLFLAG_DYN) | if (oid->oid_kind & CTLFLAG_DYN) | ||||
atomic_add_int(&oid->oid_running, 1); | atomic_add_int(&oid->oid_running, 1); | ||||
if (tracker != NULL) | if (tracker != NULL) | ||||
SYSCTL_RUNLOCK(tracker); | SYSCTL_RUNLOCK(tracker); | ||||
else | else | ||||
SYSCTL_WUNLOCK(); | SYSCTL_WUNLOCK(); | ||||
if (!(oid->oid_kind & CTLFLAG_MPSAFE)) | /* | ||||
* Treat set CTLFLAG_NEEDGIANT and unset CTLFLAG_MPSAFE flags the same, | |||||
* untill we're ready to remove all traces of Giant from sysctl(9). | |||||
*/ | |||||
if ((oid->oid_kind & CTLFLAG_NEEDGIANT) || | |||||
(!(oid->oid_kind & CTLFLAG_MPSAFE))) | |||||
mtx_lock(&Giant); | mtx_lock(&Giant); | ||||
error = oid->oid_handler(oid, arg1, arg2, req); | error = oid->oid_handler(oid, arg1, arg2, req); | ||||
if (!(oid->oid_kind & CTLFLAG_MPSAFE)) | if ((oid->oid_kind & CTLFLAG_NEEDGIANT) || | ||||
(!(oid->oid_kind & CTLFLAG_MPSAFE))) | |||||
mtx_unlock(&Giant); | mtx_unlock(&Giant); | ||||
KFAIL_POINT_ERROR(_debug_fail_point, sysctl_running, error); | KFAIL_POINT_ERROR(_debug_fail_point, sysctl_running, error); | ||||
if (tracker != NULL) | if (tracker != NULL) | ||||
SYSCTL_RLOCK(tracker); | SYSCTL_RLOCK(tracker); | ||||
else | else | ||||
SYSCTL_WLOCK(); | SYSCTL_WLOCK(); | ||||
▲ Show 20 Lines • Show All 727 Lines • ▼ Show 20 Lines | |||||
SET_DECLARE(sysctl_set, struct sysctl_oid); | SET_DECLARE(sysctl_set, struct sysctl_oid); | ||||
static void | static void | ||||
sysctl_register_all(void *arg) | sysctl_register_all(void *arg) | ||||
{ | { | ||||
struct sysctl_oid **oidp; | struct sysctl_oid **oidp; | ||||
sx_init(&sysctlmemlock, "sysctl mem"); | sx_init(&sysctlmemlock, "sysctl mem"); | ||||
sx_init(&sysctlstringlock, "sysctl string handler"); | |||||
SYSCTL_INIT(); | SYSCTL_INIT(); | ||||
SYSCTL_WLOCK(); | SYSCTL_WLOCK(); | ||||
SET_FOREACH(oidp, sysctl_set) | SET_FOREACH(oidp, sysctl_set) | ||||
sysctl_register_oid(*oidp); | sysctl_register_oid(*oidp); | ||||
SYSCTL_WUNLOCK(); | SYSCTL_WUNLOCK(); | ||||
} | } | ||||
SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_FIRST, sysctl_register_all, NULL); | SYSINIT(sysctl, SI_SUB_KMEM, SI_ORDER_FIRST, sysctl_register_all, NULL); | ||||
▲ Show 20 Lines • Show All 699 Lines • ▼ Show 20 Lines | |||||
* Two cases: | * Two cases: | ||||
* a variable string: point arg1 at it, arg2 is max length. | * a variable string: point arg1 at it, arg2 is max length. | ||||
* a constant string: point arg1 at it, arg2 is zero. | * a constant string: point arg1 at it, arg2 is zero. | ||||
*/ | */ | ||||
int | int | ||||
sysctl_handle_string(SYSCTL_HANDLER_ARGS) | sysctl_handle_string(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
char *tmparg; | |||||
size_t outlen; | size_t outlen; | ||||
int error = 0, ro_string = 0; | int error = 0, ro_string = 0; | ||||
/* | /* | ||||
* If the sysctl isn't writable, microoptimise and treat it as a | |||||
* const string. | |||||
* A zero-length buffer indicates a fixed size read-only | * A zero-length buffer indicates a fixed size read-only | ||||
* string. In ddb, don't worry about trying to make a malloced | * string. In ddb, don't worry about trying to make a malloced | ||||
* snapshot. | * snapshot. | ||||
*/ | */ | ||||
if (arg2 == 0 || kdb_active) { | if (!(oidp->oid_kind & CTLFLAG_WR) || arg2 == 0 || kdb_active) { | ||||
arg2 = strlen((char *)arg1) + 1; | arg2 = strlen((char *)arg1) + 1; | ||||
ro_string = 1; | ro_string = 1; | ||||
} | } | ||||
if (req->oldptr != NULL) { | if (req->oldptr != NULL) { | ||||
char *tmparg; | |||||
if (ro_string) { | if (ro_string) { | ||||
tmparg = arg1; | tmparg = arg1; | ||||
outlen = strlen(tmparg) + 1; | |||||
} else { | } else { | ||||
/* try to make a coherent snapshot of the string */ | |||||
tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK); | tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK); | ||||
sx_slock(&sysctlstringlock); | |||||
memcpy(tmparg, arg1, arg2); | memcpy(tmparg, arg1, arg2); | ||||
sx_sunlock(&sysctlstringlock); | |||||
kib: What is the point of using strnlen ?
Also, why do you need to call it under the lock ? | |||||
outlen = strlen(tmparg) + 1; | |||||
} | } | ||||
outlen = strnlen(tmparg, arg2 - 1) + 1; | |||||
error = SYSCTL_OUT(req, tmparg, outlen); | error = SYSCTL_OUT(req, tmparg, outlen); | ||||
if (!ro_string) | if (!ro_string) | ||||
free(tmparg, M_SYSCTLTMP); | free(tmparg, M_SYSCTLTMP); | ||||
} else { | } else { | ||||
outlen = strnlen((char *)arg1, arg2 - 1) + 1; | outlen = strlen((char *)arg1) + 1; | ||||
kibUnsubmitted Not Done Inline ActionsDon't this strlen call requires the same locking for !ro_string ? Otherwise a parallel update would cause UB. kib: Don't this strlen call requires the same locking for !ro_string ? Otherwise a parallel update… | |||||
error = SYSCTL_OUT(req, NULL, outlen); | error = SYSCTL_OUT(req, NULL, outlen); | ||||
} | } | ||||
Not Done Inline ActionsDoesn't this unlock allos outlen to become invalid, if a parallel modification is performed ? I believe that the use of SYSCTL_OUT below would imply that. kib: Doesn't this unlock allos outlen to become invalid, if a parallel modification is performed ? | |||||
if (error || !req->newptr) | if (error || !req->newptr) | ||||
Not Done Inline ActionsAgain, why is SYSCTL_OUT is called under the lock ? kib: Again, why is SYSCTL_OUT is called under the lock ? | |||||
return (error); | return (error); | ||||
if ((req->newlen - req->newidx) >= arg2) { | if ((req->newlen - req->newidx) >= arg2 || | ||||
(req->newlen - req->newidx) <= 0) { | |||||
kibUnsubmitted Not Done Inline ActionsDrop excessive () in both lines kib: Drop excessive () in both lines | |||||
error = EINVAL; | error = EINVAL; | ||||
} else { | } else { | ||||
arg2 = (req->newlen - req->newidx); | arg2 = req->newlen - req->newidx; | ||||
error = SYSCTL_IN(req, arg1, arg2); | tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK); | ||||
error = copyin((const char *)req->newptr + req->newidx, | |||||
tmparg, arg2); | |||||
if (error) { | |||||
free(tmparg, M_SYSCTLTMP); | |||||
return (error); | |||||
} | |||||
sx_xlock(&sysctlstringlock); | |||||
memcpy(arg1, tmparg, arg2); | |||||
Not Done Inline ActionsI still do not like this. In particular, I do not like that for non-wired case, an error from SYSCTL_IN leaves the in-kernel string undefined, for instance if part of it was copied in, but part is on faulting page. Might be, malloc the buffer, do copyin, then memcpy from the buffer to the final place under the lock, similar to the read case. It would also remove ordering between sysctlstringlock and all VFS/VM/net locks. kib: I still do not like this. In particular, I do not like that for non-wired case, an error from… | |||||
((char *)arg1)[arg2] = '\0'; | ((char *)arg1)[arg2] = '\0'; | ||||
sx_xunlock(&sysctlstringlock); | |||||
free(tmparg, M_SYSCTLTMP); | |||||
req->newidx += arg2; | |||||
} | } | ||||
return (error); | return (error); | ||||
} | } | ||||
/* | /* | ||||
* Handle any kind of opaque data. | * Handle any kind of opaque data. | ||||
* arg1 points to it, arg2 is the size. | * arg1 points to it, arg2 is the size. | ||||
*/ | */ | ||||
▲ Show 20 Lines • Show All 1,174 Lines • Show Last 20 Lines |
What is the point of using strnlen ?
Also, why do you need to call it under the lock ?