Patchwork [3/7] tunables: Add hooks to get and update tunables

login
register
mail settings
Submitter Siddhesh Poyarekar
Date May 11, 2017, 2:51 p.m.
Message ID <1494514306-4167-4-git-send-email-siddhesh@sourceware.org>
Download mbox | patch
Permalink /patch/20399/
State New
Headers show

Comments

Siddhesh Poyarekar - May 11, 2017, 2:51 p.m.
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.

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.

	* 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(-)
Adhemerval Zanella Netto - May 16, 2017, 8:29 p.m.
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.  */
>
Siddhesh Poyarekar - May 17, 2017, 7:17 a.m.
On Wednesday 17 May 2017 01:59 AM, Adhemerval Zanella wrote:
> 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 }); \	
>    })

It already does the range check based on the type of the tunable using
the TUNABLE_SET_VAL_IF_VALID_RANGE macro.  Isn't that sufficient?

>> 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?

The __tunable_update_val function is currently private to ld.so, so the
likelihood of doing an update outside of the linker (and hence after
relocation) is remote.  The only possibility is for the possibility of
the tunable being updated inside ld.so after relocation, but that is the
case for GLRO(...) vars too.

Maybe something for us to note in future is to ensure that if/when we
introduce tunables that can be updated at runtime, we ensure that they
have different accessors from the regular tunables that cannot touch the
immutable tunables.

Siddhesh
Adhemerval Zanella Netto - May 17, 2017, 1:52 p.m.
On 17/05/2017 04:17, Siddhesh Poyarekar wrote:
> On Wednesday 17 May 2017 01:59 AM, Adhemerval Zanella wrote:
>> 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 }); \	
>>    })
> 
> It already does the range check based on the type of the tunable using
> the TUNABLE_SET_VAL_IF_VALID_RANGE macro.  Isn't that sufficient?

Indeed and I think it might be extensible enough for user-defined types
(for instance a tunable that might have a very strict value range).  In
any case, I still think to use the value instead of a pointer would be
simpler, unless you plan to TUNABLE_UPDATE_VAL to return the previous
value (which seems to the case currently).

> 
>>> 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?
> 
> The __tunable_update_val function is currently private to ld.so, so the
> likelihood of doing an update outside of the linker (and hence after
> relocation) is remote.  The only possibility is for the possibility of
> the tunable being updated inside ld.so after relocation, but that is the
> case for GLRO(...) vars too.
> 
> Maybe something for us to note in future is to ensure that if/when we
> introduce tunables that can be updated at runtime, we ensure that they
> have different accessors from the regular tunables that cannot touch the
> immutable tunables.

Fair enough.

Patch

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