Message ID | 20170926183807.31788-1-amakhalov@vmware.com |
---|---|
State | New |
Headers | show |
Series | Fix range check in do_tunable_update_val | expand |
On Wednesday 27 September 2017 12:08 AM, Alexey Makhalov wrote: > Current implementation of tunables does not set arena_max and arena_test > values. Any value provided by glibc.malloc.arena_max and > glibc.malloc.arena_test parameters is ignored. > > These tunables have minval value set to 1 (see elf/dl-tunables.list file) > and undefined maxval value. In that case default value (which is 0. see > scripts/gen-tunables.awk) is being used to set maxval. > > For instance, generated tunable_list[] entry for arena_max is: > (gdb) p *cur > $1 = {name = 0x7ffff7df6217 "glibc.malloc.arena_max", > type = {type_code = TUNABLE_TYPE_SIZE_T, min = 1, max = 0}, > val = {numval = 0, strval = 0x0}, initialized = false, > security_level = TUNABLE_SECLEVEL_SXID_IGNORE, > env_alias = 0x7ffff7df622e "MALLOC_ARENA_MAX"} > > As a result, any value of glibc.malloc.arena_max is ignored by > TUNABLE_SET_VAL_IF_VALID_RANGE macro > __type min = (__cur)->type.min; <- initialized to 1 > __type max = (__cur)->type.max; <- initialized to 0! > if (min == max) <- false > { > min = __default_min; > max = __default_max; > } > if ((__type) (__val) >= min && (__type) (val) <= max) <- false > { > (__cur)->val.numval = val; > (__cur)->initialized = true; > } > > Assigning correct min/max values at a build time fixes a problem. > Plus, a bit of optimization: Setting of default min/max values for the > given type at a run time might be eliminated. Thanks for the patch. Can you please clarify the status of your copyright assignment (and Carlos verify it please)? I don't have access to the copyright assignment list, so once that question is cleared, I'll test and merge this patch. Siddhesh
On 09/26/2017 12:54 PM, Siddhesh Poyarekar wrote: > On Wednesday 27 September 2017 12:08 AM, Alexey Makhalov wrote: >> Current implementation of tunables does not set arena_max and arena_test >> values. Any value provided by glibc.malloc.arena_max and >> glibc.malloc.arena_test parameters is ignored. >> >> These tunables have minval value set to 1 (see elf/dl-tunables.list file) >> and undefined maxval value. In that case default value (which is 0. see >> scripts/gen-tunables.awk) is being used to set maxval. >> >> For instance, generated tunable_list[] entry for arena_max is: >> (gdb) p *cur >> $1 = {name = 0x7ffff7df6217 "glibc.malloc.arena_max", >> type = {type_code = TUNABLE_TYPE_SIZE_T, min = 1, max = 0}, >> val = {numval = 0, strval = 0x0}, initialized = false, >> security_level = TUNABLE_SECLEVEL_SXID_IGNORE, >> env_alias = 0x7ffff7df622e "MALLOC_ARENA_MAX"} >> >> As a result, any value of glibc.malloc.arena_max is ignored by >> TUNABLE_SET_VAL_IF_VALID_RANGE macro >> __type min = (__cur)->type.min; <- initialized to 1 >> __type max = (__cur)->type.max; <- initialized to 0! >> if (min == max) <- false >> { >> min = __default_min; >> max = __default_max; >> } >> if ((__type) (__val) >= min && (__type) (val) <= max) <- false >> { >> (__cur)->val.numval = val; >> (__cur)->initialized = true; >> } >> >> Assigning correct min/max values at a build time fixes a problem. >> Plus, a bit of optimization: Setting of default min/max values for the >> given type at a run time might be eliminated. > > Thanks for the patch. Can you please clarify the status of your > copyright assignment (and Carlos verify it please)? I don't have access > to the copyright assignment list, so once that question is cleared, I'll > test and merge this patch. Both Alexey and VMWare for Alexey have assignments in place. We can accept any patches from Alexey. Cheers, Carlos.
On Wednesday 27 September 2017 01:37 AM, Carlos O'Donell wrote: > Both Alexey and VMWare for Alexey have assignments in place. > > We can accept any patches from Alexey. Thanks, pushed. Siddhesh
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index b964a09..cc2c637 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -89,18 +89,11 @@ get_next_env (char **envp, char **name, size_t *namelen, char **val, return NULL; } -#define TUNABLE_SET_VAL_IF_VALID_RANGE(__cur, __val, __type, __default_min, \ - __default_max) \ +#define TUNABLE_SET_VAL_IF_VALID_RANGE(__cur, __val, __type) \ ({ \ __type min = (__cur)->type.min; \ __type max = (__cur)->type.max; \ \ - if (min == max) \ - { \ - min = __default_min; \ - max = __default_max; \ - } \ - \ if ((__type) (__val) >= min && (__type) (val) <= max) \ { \ (__cur)->val.numval = val; \ @@ -120,17 +113,17 @@ do_tunable_update_val (tunable_t *cur, const void *valp) { case TUNABLE_TYPE_INT_32: { - TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, int64_t, INT32_MIN, INT32_MAX); + TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, int64_t); break; } case TUNABLE_TYPE_UINT_64: { - TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, uint64_t, 0, UINT64_MAX); + TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, uint64_t); break; } case TUNABLE_TYPE_SIZE_T: { - TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, uint64_t, 0, SIZE_MAX); + TUNABLE_SET_VAL_IF_VALID_RANGE (cur, val, uint64_t); break; } case TUNABLE_TYPE_STRING: diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk index ccdd0c6..6221990 100644 --- a/scripts/gen-tunables.awk +++ b/scripts/gen-tunables.awk @@ -1,6 +1,14 @@ # Generate dl-tunable-list.h from dl-tunables.list BEGIN { + min_of["STRING"]="0" + max_of["STRING"]="0" + min_of["INT_32"]="INT32_MIN" + max_of["INT_32"]="INT32_MAX" + min_of["UINT_64"]="0" + max_of["UINT_64"]="UINT64_MAX" + min_of["SIZE_T"]="0" + max_of["SIZE_T"]="SIZE_MAX" tunable="" ns="" top_ns="" @@ -43,10 +51,10 @@ $1 == "}" { types[top_ns,ns,tunable] = "STRING" } if (!minvals[top_ns,ns,tunable]) { - minvals[top_ns,ns,tunable] = "0" + minvals[top_ns,ns,tunable] = min_of[types[top_ns,ns,tunable]] } if (!maxvals[top_ns,ns,tunable]) { - maxvals[top_ns,ns,tunable] = "0" + maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]] } if (!env_alias[top_ns,ns,tunable]) { env_alias[top_ns,ns,tunable] = "NULL"