From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 105511 invoked by alias); 22 May 2017 14:07:56 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 105471 invoked by uid 89); 22 May 2017 14:07:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=3757, blocker X-HELO: mail-qk0-f174.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=9tWTw8GGq8vENYLLPE8FlE2BK5Jc/oGmqes2NZ1I2zA=; b=Ih8cxqUufqbJnT1OhVdn1hU1RAZuEuaue+31vz6FVRghy7wz1oMd+9mHva8FHGlSFJ YiM+DxWZqHhY9K9szeHVkfG/fsWPVe55N+S0mWy9f6lbt8O377bu9L/qOOZpm7mak5Eg 49SsJtJaQlCeKc80KtQDTymVInDhKkKnIl47ZBwg9wOrSk5fanm+KpIfo/HW5ugog31/ poFYWE83QAYY1Ho6Ytr8GF3snTH10UNsacTjhu8WGYYBjukpqRgABECxwNRIrQ3kRGRj +25J7uMeNaGo0fEK7Lj5ufyEdCOypYryoVcf/2uzi+QF341zHRFGCH0dqVU1ZV1mrDNz Kjlg== X-Gm-Message-State: AODbwcAWkVtyHIoZgPIiNuRjaBdYL5hW3G+yOFW7LPW/RdGwzNKQfEVZ xg1AyWOakDikKi9l9+V0/g== X-Received: by 10.55.6.137 with SMTP id 131mr19644953qkg.191.1495462074295; Mon, 22 May 2017 07:07:54 -0700 (PDT) Subject: Re: [PATCH 1/5] tunables: Add hooks to get and update tunables To: Siddhesh Poyarekar , libc-alpha@sourceware.org References: <1495138038-32212-1-git-send-email-siddhesh@sourceware.org> <1495138038-32212-2-git-send-email-siddhesh@sourceware.org> From: Adhemerval Zanella Message-ID: <82bea48c-c0fe-520c-0f79-dfcd96509c5f@linaro.org> Date: Mon, 22 May 2017 14:07:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1495138038-32212-2-git-send-email-siddhesh@sourceware.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-05/txt/msg00651.txt.bz2 On 18/05/2017 17:07, 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 the value to be stored into the tunable > structure followed by the type of the value. For example: > > TUNABLE_UPDATE_VAL (glibc, malloc, check, 1, int32_t); > > 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. > * README.tunables: Document the new macros. LGTM (I added just some document extension points and it is not a patch blocker). > --- > README.tunables | 27 +++++++++++++++++++++++++++ > elf/dl-tunables.c | 43 +++++++++++++++++++++++++++++++++++-------- > elf/dl-tunables.h | 40 ++++++++++++++++++++++++++++------------ > 3 files changed, 90 insertions(+), 20 deletions(-) > > diff --git a/README.tunables b/README.tunables > index 0e9b0d7..0bea2ca 100644 > --- a/README.tunables > +++ b/README.tunables > @@ -75,6 +75,33 @@ The list of allowed attributes are: > TUNABLE_SET_VALUE_WITH_CALLBACK instead and additionally pass a pointer to > the function that should be called if the tunable value has been set. > > +GETTING A TUNABLE VALUE > +----------------------- > + > +One may get the value of a tunable anywhere in glibc code using the TUNABLE_GET > +macro as follows: > + > + TUNABLE_GET (glibc, malloc, check, int32_t) > + > +The first three arguments of the macro are the top namespace, tunable namespace > +and the tunable name respectively. The last argument is the type of the > +tunable and must match the type of the tunable. > + > +UPDATING A TUNABLE VALUE > +------------------------ > + > +The tunable list is currently stored in a relro section, i.e. it cannot be > +modified after relocation. As a result, one may update the value of a tunable > +only in the dynamic linker. The TUNABLE_UPDATE_VALUE macro can be used to do > +this: > + > + TUNABLE_UPDATE_VAL (glibc, malloc, check, val, int32_t); > + > +where the first thre arguments of the macro are the top namespace, tunable > +namespace and the tunable name respectively. The fourth argument is the value > +to assign to the tunable and the last argument is the type of the tunable. The > +type in the fifth argument must match the type of the tunable. > + Wouldn't be worth do point where in code it updates now? The idea would just to add some reference point for possible future work. > FUTURE WORK > ----------- > > 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..f465370 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,29 +61,45 @@ 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_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, __type) \ > ({ \ > - __tunable_set_val \ > - (TUNABLE_ENUM_NAME (TOP_NAMESPACE, TUNABLE_NAMESPACE, __id), (__val), \ > - DL_TUNABLE_CALLBACK (__cb)); \ > + __tunable_update_val (TUNABLE_ENUM_NAME (__top, __ns, __id), \ > + & (__type) {__val}); \ > }) > > /* Namespace sanity for callback functions. Use this macro to keep the >