Fix range check in do_tunable_update_val
Commit Message
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.
* elf/dl-tunables.c (do_tunable_update_val): Range checking fix.
* scripts/gen-tunables.awk: Set unspecified minval and/or maxval
values to correct default value for given type.
---
elf/dl-tunables.c | 15 ++++-----------
scripts/gen-tunables.awk | 12 ++++++++++--
2 files changed, 14 insertions(+), 13 deletions(-)
Comments
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
@@ -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:
@@ -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"