This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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.  */
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]