This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 3/7] tunables: Add hooks to get and update tunables
On 11/05/2017 11:51, Siddhesh Poyarekar wrote:
> This change adds two new tunable macros to get and update tunables
> respectively called TUNABLE_GET and TUNABLE_UPDATE_VAL.
>
> TUNABLE_GET accepts the top namespace, tunable namespace and tunable
> id along with the target type as arguments and returns a value of the
> target type. For example:
>
> TUNABLE_GET (glibc, malloc, check, int32_t)
>
> will return the tunable value for glibc.malloc.check in an int32_t.
>
> TUNABLE_UPDATE_VAL accepts the top namespace, tunable namespace and
> tunable id along with a pointer to the value to be stored into the
> tunable structure. For example:
>
> int32_t val = 1;
> TUNABLE_UPDATE_VAL (glibc, malloc, check, &val);
>
> will update the glibc.malloc.check tunable.
I would prefer to simplify a bit this macro to avoid use pointer and
use the value instead and specify the type directly (so we can add
some range/type check in the future). Something like:
# define TUNABLE_UPDATE_VAL(__top, __ns, __id, __val, __type) \
({ \
__tunable_update_val (TUNABLE_ENUM_NAME (__top, __ns, __id), \
&(__type) { __val }); \
})
>
> Given that the tunable structure is now relro, this is only really
> possible from within the dynamic linker before it is relocated. In
> future the tunable list may get split into mutable and immutable
> tunables where mutable tunables can be modified by the library and
> userspace after relocation as well. However whenever we actually do
> that, we will have to ensure that the mutable tunables are protected
> with locks.
This worries me a bit because I would like a compiler/linker warning
that updating a tunable in wrong place is not allowed. Is there a
way to make compiler/linker barf with an invalid update?
>
> * elf/dl-tunables.h (strval): Replace with a bool initialized.
> (TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
> prevent collision.
> (__tunable_update_val): New function.
> (TUNABLE_GET): New macro.
> (TUNABLE_SET_VAL_FULL_WITH_CALLBACK): New macro.
> (TUNABLE_SET_VAL): Use it.
> (TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
> (TUNABLE_UPDATE_VAL): New macro.
> (tunables_strtoul): Replace strval with initialized.
> (__tunables_init): Likewise.
> (__tunable_set_val): Likewise.
> (do_tunable_update_val): New function.
> (tunables_initialize): Use it.
> (__tunable_update_val): New function.
> ---
> elf/dl-tunables.c | 43 +++++++++++++++++++++++++++++++++++--------
> elf/dl-tunables.h | 41 +++++++++++++++++++++++++++--------------
> 2 files changed, 62 insertions(+), 22 deletions(-)
>
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 8d72e26..abf10e5 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -184,19 +184,17 @@ tunables_strtoul (const char *nptr)
> if ((__type) (__val) >= min && (__type) (val) <= max) \
> { \
> (__cur)->val.numval = val; \
> - (__cur)->strval = strval; \
> + (__cur)->initialized = true; \
> } \
> })
>
> -/* Validate range of the input value and initialize the tunable CUR if it looks
> - good. */
> static void
> -tunable_initialize (tunable_t *cur, const char *strval)
> +do_tunable_update_val (tunable_t *cur, const void *valp)
> {
> uint64_t val;
>
> if (cur->type.type_code != TUNABLE_TYPE_STRING)
> - val = tunables_strtoul (strval);
> + val = *((int64_t *) valp);
>
> switch (cur->type.type_code)
> {
> @@ -217,7 +215,7 @@ tunable_initialize (tunable_t *cur, const char *strval)
> }
> case TUNABLE_TYPE_STRING:
> {
> - cur->val.strval = cur->strval = strval;
> + cur->val.strval = valp;
> break;
> }
> default:
> @@ -225,6 +223,35 @@ tunable_initialize (tunable_t *cur, const char *strval)
> }
> }
>
> +/* Validate range of the input value and initialize the tunable CUR if it looks
> + good. */
> +static void
> +tunable_initialize (tunable_t *cur, const char *strval)
> +{
> + uint64_t val;
> + const void *valp;
> +
> + if (cur->type.type_code != TUNABLE_TYPE_STRING)
> + {
> + val = tunables_strtoul (strval);
> + valp = &val;
> + }
> + else
> + {
> + cur->initialized = true;
> + valp = strval;
> + }
> + do_tunable_update_val (cur, valp);
> +}
> +
> +void
> +__tunable_update_val (tunable_id_t id, void *valp)
> +{
> + tunable_t *cur = &tunable_list[id];
> +
> + do_tunable_update_val (cur, valp);
> +}
> +
> #if TUNABLES_FRONTEND == TUNABLES_FRONTEND_valstring
> /* Parse the tunable string TUNESTR and adjust it to drop any tunables that may
> be unsafe for AT_SECURE processes so that it can be used as the new
> @@ -375,7 +402,7 @@ __tunables_init (char **envp)
>
> /* Skip over tunables that have either been set already or should be
> skipped. */
> - if (cur->strval != NULL || cur->env_alias == NULL)
> + if (cur->initialized || cur->env_alias == NULL)
> continue;
>
> const char *name = cur->env_alias;
> @@ -432,7 +459,7 @@ __tunable_set_val (tunable_id_t id, void *valp, tunable_callback_t callback)
>
> /* Don't do anything if our tunable was not set during initialization or if
> it failed validation. */
> - if (cur->strval == NULL)
> + if (!cur->initialized)
> return;
>
> /* Caller does not need the value, just call the callback with our tunable
> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index f33adfb..7a1124a 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -39,8 +39,8 @@ struct _tunable
> const char *name; /* Internal name of the tunable. */
> tunable_type_t type; /* Data type of the tunable. */
> tunable_val_t val; /* The value. */
> - const char *strval; /* The string containing the value,
> - points into envp. */
> + bool initialized; /* Flag to indicate that the tunable is
> + initialized. */
> tunable_seclevel_t security_level; /* Specify the security level for the
> tunable with respect to AT_SECURE
> programs. See description of
> @@ -61,30 +61,43 @@ typedef struct _tunable tunable_t;
> /* Full name for a tunable is top_ns.tunable_ns.id. */
> # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id
>
> -# define TUNABLE_ENUM_NAME(top,ns,id) TUNABLE_ENUM_NAME1 (top,ns,id)
> -# define TUNABLE_ENUM_NAME1(top,ns,id) top ## _ ## ns ## _ ## id
> +# define TUNABLE_ENUM_NAME(__top,__ns,__id) TUNABLE_ENUM_NAME1 (__top,__ns,__id)
> +# define TUNABLE_ENUM_NAME1(__top,__ns,__id) __top ## _ ## __ns ## _ ## __id
>
> # include "dl-tunable-list.h"
>
> extern void __tunables_init (char **);
> extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t);
> +extern void __tunable_update_val (tunable_id_t, void *);
> +
> +/* Get and return a tunable value. */
> +# define TUNABLE_GET(__top, __ns, __id, __type) \
> +({ \
> + tunable_id_t id = TUNABLE_ENUM_NAME (__top, __ns, __id); \
> + __type ret; \
> + TUNABLE_SET_VAL_FULL_WITH_CALLBACK (id, &ret, NULL); \
> + ret; \
> +})
>
> /* Check if the tunable has been set to a non-default value and if it is, copy
> it over into __VAL. */
> # define TUNABLE_SET_VAL(__id,__val) \
> -({ \
> - __tunable_set_val \
> - (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \
> - NULL); \
> -})
> + TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE, \
> + TUNABLE_NAMESPACE, \
> + __id), (__val), NULL)
>
> /* Same as TUNABLE_SET_VAL, but also call the callback function __CB. */
> # define TUNABLE_SET_VAL_WITH_CALLBACK(__id,__val,__cb) \
> -({ \
> - __tunable_set_val \
> - (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \
> - DL_TUNABLE_CALLBACK (__cb)); \
> -})
> + TUNABLE_SET_VAL_FULL_WITH_CALLBACK (TUNABLE_ENUM_NAME (TOP_NAMESPACE, \
> + TUNABLE_NAMESPACE, \
> + __id), \
> + (__val), DL_TUNABLE_CALLBACK (__cb))
> +
> +# define TUNABLE_SET_VAL_FULL_WITH_CALLBACK(__id,__val,__cb)\
> + __tunable_set_val (__id, (__val), (__cb))
> +
> +# define TUNABLE_UPDATE_VAL(__top, __ns, __id, __val) \
> + __tunable_update_val (TUNABLE_ENUM_NAME (__top, __ns, __id), __val)
>
> /* Namespace sanity for callback functions. Use this macro to keep the
> namespace of the modules clean. */
>