[v5,2/5] elf: Do not set invalid tunables values
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
The loader now warns for invalid and out-of-range tunable values. The
patch also fixes the parsing of size_t maximum values, where
_dl_strtoul was failing for large values close to SIZE_MAX.
Checked on x86_64-linux-gnu.
---
elf/dl-misc.c | 5 ++++-
elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 64 insertions(+), 6 deletions(-)
Comments
On 2023-11-22 15:43, Adhemerval Zanella wrote:
> The loader now warns for invalid and out-of-range tunable values. The
> patch also fixes the parsing of size_t maximum values, where
> _dl_strtoul was failing for large values close to SIZE_MAX.
>
> Checked on x86_64-linux-gnu.
> ---
> elf/dl-misc.c | 5 ++++-
> elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
> elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/elf/dl-misc.c b/elf/dl-misc.c
> index 5b84adc2f4..037cbb3650 100644
> --- a/elf/dl-misc.c
> +++ b/elf/dl-misc.c
> @@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
> }
> }
>
> + const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
> + const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
> +
> while (1)
> {
> int digval;
> @@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
> else
> break;
>
> - if (result >= (UINT64_MAX - digval) / base)
> + if (result > cutoff || (result == cutoff && digval > cutlim))
I don't understand this change; how does this work with octal or
hexadecimal inputs?
> {
> if (endptr != NULL)
> *endptr = (char *) nptr;
> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 26161c68e7..67a37ff704 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
> {
> tunable_num_t val, min, max;
>
> - if (cur->type.type_code == TUNABLE_TYPE_STRING)
> + switch (cur->type.type_code)
> {
> + case TUNABLE_TYPE_STRING:
> cur->val.strval = valp->strval;
> cur->initialized = true;
> return;
> + case TUNABLE_TYPE_INT_32:
> + val = (int32_t) valp->numval;
> + break;
> + case TUNABLE_TYPE_UINT_64:
> + val = (int64_t) valp->numval;
> + break;
> + case TUNABLE_TYPE_SIZE_T:
> + val = (size_t) valp->numval;
> + break;
> + default:
> + __builtin_unreachable ();
> }
>
> bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
>
> - val = valp->numval;
> min = minp != NULL ? *minp : cur->type.min;
> max = maxp != NULL ? *maxp : cur->type.max;
>
> @@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>
> /* Validate range of the input value and initialize the tunable CUR if it looks
> good. */
> -static void
> +static bool
> tunable_initialize (tunable_t *cur, const char *strval, size_t len)
> {
> tunable_val_t val = { 0 };
>
> if (cur->type.type_code != TUNABLE_TYPE_STRING)
> - val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
> + {
> + char *endptr = NULL;
> + uint64_t numval = _dl_strtoul (strval, &endptr);
> + if (endptr != strval + len)
> + return false;
> + val.numval = (tunable_num_t) numval;
> + }
> else
> val.strval = (struct tunable_str_t) { strval, len };
> do_tunable_update_val (cur, &val, NULL, NULL);
> +
> + return true;
> }
>
> void
> @@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
> }
>
> for (int i = 0; i < ntunables; i++)
> - tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
> + if (!tunable_initialize (tunables[i].t, tunables[i].value,
> + tunables[i].len))
> + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
> + "for option `%s': ignored.\n",
> + (int) tunables[i].len,
> + tunables[i].value,
> + tunables[i].t->name);
> }
>
> /* Initialize the tunables list from the environment. For now we only use the
> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
> index 188345b070..d6a1e1b3ac 100644
> --- a/elf/tst-tunables.c
> +++ b/elf/tst-tunables.c
> @@ -53,6 +53,13 @@ static const struct test_t
> 4096,
> 0,
> },
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.mmap_threshold=-1",
> + 0,
> + SIZE_MAX,
> + 0,
> + },
> /* Empty tunable are ignored. */
> {
> "GLIBC_TUNABLES",
> @@ -224,6 +231,29 @@ static const struct test_t
> 0,
> 0,
> },
> + /* Invalid numbers are ignored. */
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
> + 0,
> + 4096,
> + 0,
> + },
> + {
> + "GLIBC_TUNABLES",
> + "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
> + 2,
> + 0,
> + 0,
> + },
> + {
> + "GLIBC_TUNABLES",
> + /* SIZE_MAX + 1 */
> + "glibc.malloc.mmap_threshold=18446744073709551616",
> + 0,
> + 0,
> + 0,
> + },
> /* Also check some tunable aliases. */
> {
> "MALLOC_CHECK_",
On 01/12/23 12:32, Siddhesh Poyarekar wrote:
>
>
> On 2023-11-22 15:43, Adhemerval Zanella wrote:
>> The loader now warns for invalid and out-of-range tunable values. The
>> patch also fixes the parsing of size_t maximum values, where
>> _dl_strtoul was failing for large values close to SIZE_MAX.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>> elf/dl-misc.c | 5 ++++-
>> elf/dl-tunables.c | 35 ++++++++++++++++++++++++++++++-----
>> elf/tst-tunables.c | 30 ++++++++++++++++++++++++++++++
>> 3 files changed, 64 insertions(+), 6 deletions(-)
>>
>> diff --git a/elf/dl-misc.c b/elf/dl-misc.c
>> index 5b84adc2f4..037cbb3650 100644
>> --- a/elf/dl-misc.c
>> +++ b/elf/dl-misc.c
>> @@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
>> }
>> }
>> + const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
>> + const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
>> +
>> while (1)
>> {
>> int digval;
>> @@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
>> else
>> break;
>> - if (result >= (UINT64_MAX - digval) / base)
>> + if (result > cutoff || (result == cutoff && digval > cutlim))
>
> I don't understand this change; how does this work with octal or hexadecimal inputs?
In fact the cutoff/cutlim should be adjusted when a different base is used,
I will fix it. The logic here is similar to the stdlib/strtol_l.c:486.
>
>> {
>> if (endptr != NULL)
>> *endptr = (char *) nptr;
>> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
>> index 26161c68e7..67a37ff704 100644
>> --- a/elf/dl-tunables.c
>> +++ b/elf/dl-tunables.c
>> @@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>> {
>> tunable_num_t val, min, max;
>> - if (cur->type.type_code == TUNABLE_TYPE_STRING)
>> + switch (cur->type.type_code)
>> {
>> + case TUNABLE_TYPE_STRING:
>> cur->val.strval = valp->strval;
>> cur->initialized = true;
>> return;
>> + case TUNABLE_TYPE_INT_32:
>> + val = (int32_t) valp->numval;
>> + break;
>> + case TUNABLE_TYPE_UINT_64:
>> + val = (int64_t) valp->numval;
>> + break;
>> + case TUNABLE_TYPE_SIZE_T:
>> + val = (size_t) valp->numval;
>> + break;
>> + default:
>> + __builtin_unreachable ();
>> }
>> bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
>> - val = valp->numval;
>> min = minp != NULL ? *minp : cur->type.min;
>> max = maxp != NULL ? *maxp : cur->type.max;
>> @@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
>> /* Validate range of the input value and initialize the tunable CUR if it looks
>> good. */
>> -static void
>> +static bool
>> tunable_initialize (tunable_t *cur, const char *strval, size_t len)
>> {
>> tunable_val_t val = { 0 };
>> if (cur->type.type_code != TUNABLE_TYPE_STRING)
>> - val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
>> + {
>> + char *endptr = NULL;
>> + uint64_t numval = _dl_strtoul (strval, &endptr);
>> + if (endptr != strval + len)
>> + return false;
>> + val.numval = (tunable_num_t) numval;
>> + }
>> else
>> val.strval = (struct tunable_str_t) { strval, len };
>> do_tunable_update_val (cur, &val, NULL, NULL);
>> +
>> + return true;
>> }
>> void
>> @@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
>> }
>> for (int i = 0; i < ntunables; i++)
>> - tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
>> + if (!tunable_initialize (tunables[i].t, tunables[i].value,
>> + tunables[i].len))
>> + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
>> + "for option `%s': ignored.\n",
>> + (int) tunables[i].len,
>> + tunables[i].value,
>> + tunables[i].t->name);
>> }
>> /* Initialize the tunables list from the environment. For now we only use the
>> diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
>> index 188345b070..d6a1e1b3ac 100644
>> --- a/elf/tst-tunables.c
>> +++ b/elf/tst-tunables.c
>> @@ -53,6 +53,13 @@ static const struct test_t
>> 4096,
>> 0,
>> },
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.mmap_threshold=-1",
>> + 0,
>> + SIZE_MAX,
>> + 0,
>> + },
>> /* Empty tunable are ignored. */
>> {
>> "GLIBC_TUNABLES",
>> @@ -224,6 +231,29 @@ static const struct test_t
>> 0,
>> 0,
>> },
>> + /* Invalid numbers are ignored. */
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
>> + 0,
>> + 4096,
>> + 0,
>> + },
>> + {
>> + "GLIBC_TUNABLES",
>> + "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
>> + 2,
>> + 0,
>> + 0,
>> + },
>> + {
>> + "GLIBC_TUNABLES",
>> + /* SIZE_MAX + 1 */
>> + "glibc.malloc.mmap_threshold=18446744073709551616",
>> + 0,
>> + 0,
>> + 0,
>> + },
>> /* Also check some tunable aliases. */
>> {
>> "MALLOC_CHECK_",
@@ -190,6 +190,9 @@ _dl_strtoul (const char *nptr, char **endptr)
}
}
+ const uint64_t cutoff = (UINT64_MAX * 2UL + 1UL) / 10;
+ const uint64_t cutlim = (UINT64_MAX * 2UL + 1UL) % 10;
+
while (1)
{
int digval;
@@ -207,7 +210,7 @@ _dl_strtoul (const char *nptr, char **endptr)
else
break;
- if (result >= (UINT64_MAX - digval) / base)
+ if (result > cutoff || (result == cutoff && digval > cutlim))
{
if (endptr != NULL)
*endptr = (char *) nptr;
@@ -77,16 +77,27 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
{
tunable_num_t val, min, max;
- if (cur->type.type_code == TUNABLE_TYPE_STRING)
+ switch (cur->type.type_code)
{
+ case TUNABLE_TYPE_STRING:
cur->val.strval = valp->strval;
cur->initialized = true;
return;
+ case TUNABLE_TYPE_INT_32:
+ val = (int32_t) valp->numval;
+ break;
+ case TUNABLE_TYPE_UINT_64:
+ val = (int64_t) valp->numval;
+ break;
+ case TUNABLE_TYPE_SIZE_T:
+ val = (size_t) valp->numval;
+ break;
+ default:
+ __builtin_unreachable ();
}
bool unsigned_cmp = unsigned_tunable_type (cur->type.type_code);
- val = valp->numval;
min = minp != NULL ? *minp : cur->type.min;
max = maxp != NULL ? *maxp : cur->type.max;
@@ -117,16 +128,24 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
/* Validate range of the input value and initialize the tunable CUR if it looks
good. */
-static void
+static bool
tunable_initialize (tunable_t *cur, const char *strval, size_t len)
{
tunable_val_t val = { 0 };
if (cur->type.type_code != TUNABLE_TYPE_STRING)
- val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
+ {
+ char *endptr = NULL;
+ uint64_t numval = _dl_strtoul (strval, &endptr);
+ if (endptr != strval + len)
+ return false;
+ val.numval = (tunable_num_t) numval;
+ }
else
val.strval = (struct tunable_str_t) { strval, len };
do_tunable_update_val (cur, &val, NULL, NULL);
+
+ return true;
}
void
@@ -226,7 +245,13 @@ parse_tunables (const char *valstring)
}
for (int i = 0; i < ntunables; i++)
- tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
+ if (!tunable_initialize (tunables[i].t, tunables[i].value,
+ tunables[i].len))
+ _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' "
+ "for option `%s': ignored.\n",
+ (int) tunables[i].len,
+ tunables[i].value,
+ tunables[i].t->name);
}
/* Initialize the tunables list from the environment. For now we only use the
@@ -53,6 +53,13 @@ static const struct test_t
4096,
0,
},
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.mmap_threshold=-1",
+ 0,
+ SIZE_MAX,
+ 0,
+ },
/* Empty tunable are ignored. */
{
"GLIBC_TUNABLES",
@@ -224,6 +231,29 @@ static const struct test_t
0,
0,
},
+ /* Invalid numbers are ignored. */
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.check=abc:glibc.malloc.mmap_threshold=4096",
+ 0,
+ 4096,
+ 0,
+ },
+ {
+ "GLIBC_TUNABLES",
+ "glibc.malloc.check=2:glibc.malloc.mmap_threshold=abc",
+ 2,
+ 0,
+ 0,
+ },
+ {
+ "GLIBC_TUNABLES",
+ /* SIZE_MAX + 1 */
+ "glibc.malloc.mmap_threshold=18446744073709551616",
+ 0,
+ 0,
+ 0,
+ },
/* Also check some tunable aliases. */
{
"MALLOC_CHECK_",