aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJason A. Harmening <jah@FreeBSD.org>2020-08-14 21:37:38 +0000
committerAlexander Motin <mav@FreeBSD.org>2021-02-17 17:52:24 +0000
commiteefc7f388ab66a79f99a5f73236e9fbfc5312d4f (patch)
tree6e41b7563327518c412a76e4cddb51926f433acc
parent9cc7bd9a6ab1e2bfc12dabf4d9ef4ad463a05616 (diff)
downloadsrc-eefc7f388ab66a79f99a5f73236e9fbfc5312d4f.tar.gz
src-eefc7f388ab66a79f99a5f73236e9fbfc5312d4f.zip
kenv: avoid sleepable alloc for integer tunables
Avoid performing a potentially-blocking malloc for kenv lookups that will only perform non-destructive integer conversions on the returned buffer. Instead, perform the strtoq() in-place with the kenv lock held. While here, factor the logic around kenv_lock acquire and release into kenv_acquire() and kenv_release(), and use these functions for some light cleanup. Collapse getenv_string_buffer() into kern_getenv(), as the former no longer has any other callers and the only additional task performed by the latter is a WITNESS check that hasn't been useful since r362231. PR: 248250 Reported by: gbe Reviewed by: mjg Tested by: gbe Differential Revision: https://reviews.freebsd.org/D26010
-rw-r--r--sys/kern/kern_environment.c124
1 files changed, 67 insertions, 57 deletions
diff --git a/sys/kern/kern_environment.c b/sys/kern/kern_environment.c
index f2d7dcb1de4d..761113f108ec 100644
--- a/sys/kern/kern_environment.c
+++ b/sys/kern/kern_environment.c
@@ -59,6 +59,9 @@ __FBSDID("$FreeBSD$");
static char *_getenv_dynamic_locked(const char *name, int *idx);
static char *_getenv_dynamic(const char *name, int *idx);
+static char *kenv_acquire(const char *name);
+static void kenv_release(const char *buf);
+
static MALLOC_DEFINE(M_KENV, "kenv", "kernel environment");
#define KENV_SIZE 512 /* Maximum number of environment strings */
@@ -88,8 +91,6 @@ bool dynamic_kenv;
#define KENV_CHECK if (!dynamic_kenv) \
panic("%s: called before SI_SUB_KMEM", __func__)
-static char *getenv_string_buffer(const char *);
-
int
sys_kenv(td, uap)
struct thread *td;
@@ -482,16 +483,24 @@ _getenv_static(const char *name)
char *
kern_getenv(const char *name)
{
- char *ret;
+ char *cp, *ret;
+ int len;
if (dynamic_kenv) {
- ret = getenv_string_buffer(name);
- if (ret == NULL) {
- WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL,
- "getenv");
+ len = KENV_MNAMELEN + 1 + kenv_mvallen + 1;
+ ret = uma_zalloc(kenv_zone, M_WAITOK | M_ZERO);
+ mtx_lock(&kenv_lock);
+ cp = _getenv_dynamic(name, NULL);
+ if (cp != NULL)
+ strlcpy(ret, cp, len);
+ mtx_unlock(&kenv_lock);
+ if (cp == NULL) {
+ uma_zfree(kenv_zone, ret);
+ ret = NULL;
}
} else
ret = _getenv_static(name);
+
return (ret);
}
@@ -503,12 +512,9 @@ testenv(const char *name)
{
char *cp;
- if (dynamic_kenv) {
- mtx_lock(&kenv_lock);
- cp = _getenv_dynamic(name, NULL);
- mtx_unlock(&kenv_lock);
- } else
- cp = _getenv_static(name);
+ cp = kenv_acquire(name);
+ kenv_release(cp);
+
if (cp != NULL)
return (1);
return (0);
@@ -616,30 +622,33 @@ kern_unsetenv(const char *name)
}
/*
- * Return a buffer containing the string value from an environment variable
+ * Return the internal kenv buffer for the variable name, if it exists.
+ * If the dynamic kenv is initialized and the name is present, return
+ * with kenv_lock held.
*/
static char *
-getenv_string_buffer(const char *name)
+kenv_acquire(const char *name)
{
- char *cp, *ret;
- int len;
+ char *value;
if (dynamic_kenv) {
- len = KENV_MNAMELEN + 1 + kenv_mvallen + 1;
- ret = uma_zalloc(kenv_zone, M_WAITOK | M_ZERO);
mtx_lock(&kenv_lock);
- cp = _getenv_dynamic(name, NULL);
- if (cp != NULL)
- strlcpy(ret, cp, len);
- mtx_unlock(&kenv_lock);
- if (cp == NULL) {
- uma_zfree(kenv_zone, ret);
- ret = NULL;
- }
+ value = _getenv_dynamic(name, NULL);
+ if (value == NULL)
+ mtx_unlock(&kenv_lock);
+ return (value);
} else
- ret = _getenv_static(name);
+ return (_getenv_static(name));
+}
- return (ret);
+/*
+ * Undo a previous kenv_acquire() operation
+ */
+static void
+kenv_release(const char *buf)
+{
+ if ((buf != NULL) && dynamic_kenv)
+ mtx_unlock(&kenv_lock);
}
/*
@@ -650,17 +659,13 @@ getenv_string(const char *name, char *data, int size)
{
char *cp;
- if (dynamic_kenv) {
- mtx_lock(&kenv_lock);
- cp = _getenv_dynamic(name, NULL);
- if (cp != NULL)
- strlcpy(data, cp, size);
- mtx_unlock(&kenv_lock);
- } else {
- cp = _getenv_static(name);
- if (cp != NULL)
- strlcpy(data, cp, size);
- }
+ cp = kenv_acquire(name);
+
+ if (cp != NULL)
+ strlcpy(data, cp, size);
+
+ kenv_release(cp);
+
return (cp != NULL);
}
@@ -674,16 +679,18 @@ getenv_array(const char *name, void *pdata, int size, int *psize,
uint8_t shift;
int64_t value;
int64_t old;
- char *buf;
+ const char *buf;
char *end;
- char *ptr;
+ const char *ptr;
int n;
int rc;
- if ((buf = getenv_string_buffer(name)) == NULL)
- return (0);
-
rc = 0; /* assume failure */
+
+ buf = kenv_acquire(name);
+ if (buf == NULL)
+ goto error;
+
/* get maximum number of elements */
size /= type_size;
@@ -798,8 +805,7 @@ getenv_array(const char *name, void *pdata, int size, int *psize,
if (n != 0)
rc = 1; /* success */
error:
- if (dynamic_kenv)
- uma_zfree(kenv_zone, buf);
+ kenv_release(buf);
return (rc);
}
@@ -899,18 +905,21 @@ getenv_ulong(const char *name, unsigned long *data)
int
getenv_quad(const char *name, quad_t *data)
{
- char *value, *vtp;
- quad_t iv;
+ const char *value;
+ char suffix, *vtp;
+ quad_t iv;
- value = getenv_string_buffer(name);
- if (value == NULL)
- return (0);
+ value = kenv_acquire(name);
+ if (value == NULL) {
+ goto error;
+ }
iv = strtoq(value, &vtp, 0);
if (vtp == value || (vtp[0] != '\0' && vtp[1] != '\0')) {
- freeenv(value);
- return (0);
+ goto error;
}
- switch (vtp[0]) {
+ suffix = vtp[0];
+ kenv_release(value);
+ switch (suffix) {
case 't': case 'T':
iv *= 1024;
/* FALLTHROUGH */
@@ -925,12 +934,13 @@ getenv_quad(const char *name, quad_t *data)
case '\0':
break;
default:
- freeenv(value);
return (0);
}
- freeenv(value);
*data = iv;
return (1);
+error:
+ kenv_release(value);
+ return (0);
}
/*