From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id 021973858D39 for ; Wed, 6 Dec 2023 13:06:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 021973858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 021973858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::436 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701868015; cv=none; b=XZufsAeZHmTTe3a1XYPsRcF/WrKsixHJ/1dn676l47wsYJxG/8R6tC4rjWY4qh0b4DAtzPSdbpq3lDj0dct9jyyJuTdcAe2xclgTtar2gIR8iJ2mJf1CY3wyYyeyRj1vqCgRQNdvKepOiFGzjhlaGYfKKmmZy8srBZW5Zr7TFMM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701868015; c=relaxed/simple; bh=l8v3xGVrmQCkx18pEOSXZbRKULh2hKovy7ELLi/NS/c=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=t3JcO3XUgNUdYEPgyaHDt8aVhmB+ScfX/aWBNY1inHMDQizpeKtqyyfFUvyxzmiJ/eirioKnJWDkf0b2yTHUR9UuCYYz1kdGeq+W0209yMFBjveAEyflu21IGUvGLXhqm//DFMnaeyiT+bGa/4p62U9KvXxNG0472kEmdAnGe3Q= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-6ce934e9d51so383833b3a.1 for ; Wed, 06 Dec 2023 05:06:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1701868004; x=1702472804; darn=sourceware.org; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=lFEz1M+RI0EtuOJFOY5olfbKxjjQ+okD4iW4tna9xQo=; b=zeXmuFChBgDGahqT0Jujjt8rYufZNu9N1FQQAW5wdnqjtA6DAqHui5DdK4GRRIftWL 8iUwkHFN4WvbpqRiEkHEYyb+tMq+45TI2XlVGucMqOpUL5n0pHKZFZXhM3ACOJbn47Of mrW+iuN7V9Q9y8NjPBdqOrwSRaHHxYL+XOMu0Q+M4K2eGeMvjR4PnnH5XkwNYYTOqmbH qkugdwnqAkUCmTlOJ7jtxaABeCQKWHQX+qgxp7JZkMJjgGSJZPbPKLdSa8P4fdkRn4R3 toRUjd5TkfeRI6RqGZPlmUQ/KQZOl+1NjV1JybiGJIgJmw2mkEEBGrJyllaX9r/HHKOM 6jzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701868004; x=1702472804; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=lFEz1M+RI0EtuOJFOY5olfbKxjjQ+okD4iW4tna9xQo=; b=G6CnIm1wkrpF4iNCiMW6SOSeNbX1eUsPaPGloQDMfQOQYLVSOPlmoe/CkpVLkVtxsQ zQXFlkChyU0fo9kUnzEw+g2OsQCu4LEdRtL8/tV6siJH6NThp+qYQlJOBaBfbG6rsZxX Pr9pxO5Rvmr33jFv3GoeXRpZIcqOcdBGzzlVTJYkUK7DKbCEeGbtJTzgbC9z4aV5rorX xrx8f09pvZymYI2BpAGga+5Hhfd51MRXLKbedMHngenW0jSXz+JeK0O+Gtaqs8KO1Oyc asgwCatpE0pjfCScvUlnmiYZhOKfEDdINeZ9w+X6Luo+KPFRlmpR/8Bdm/ZkxM9HW9s6 5YoA== X-Gm-Message-State: AOJu0YwV7RbM3l5LbmKuzXYT3135MdVyBkIkOemqQLKqG1EBjfvBLoBu Tsc8kO50ep1ymMC6ksU9+jW6LBA7xXzRV8yf7ZA= X-Google-Smtp-Source: AGHT+IF+Bs16IgK0WBUMyTQi6Zyb1A1tJ61m6o/GE2gThp60wWee7mgTGk32EB8ZS4OjRYC9RAqfIQ== X-Received: by 2002:a05:6a21:99aa:b0:187:ce9f:e198 with SMTP id ve42-20020a056a2199aa00b00187ce9fe198mr683619pzb.5.1701868003822; Wed, 06 Dec 2023 05:06:43 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c3:56e1:647c:7ddc:4622:85e4? ([2804:1b3:a7c3:56e1:647c:7ddc:4622:85e4]) by smtp.gmail.com with ESMTPSA id d3-20020a631d03000000b005a9b20408a7sm10885392pgd.23.2023.12.06.05.06.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 Dec 2023 05:06:42 -0800 (PST) Message-ID: <4ae59d13-a48e-4e37-822e-56868026fa85@linaro.org> Date: Wed, 6 Dec 2023 10:06:40 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 2/5] elf: Do not set invalid tunables values Content-Language: en-US To: Siddhesh Poyarekar , libc-alpha@sourceware.org References: <20231122204325.4058222-1-adhemerval.zanella@linaro.org> <20231122204325.4058222-3-adhemerval.zanella@linaro.org> <97d0ba16-c463-4e26-9f48-1525dc99c28f@sourceware.org> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <97d0ba16-c463-4e26-9f48-1525dc99c28f@sourceware.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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_",