From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dormouse.elm.relay.mailchannels.net (dormouse.elm.relay.mailchannels.net [23.83.212.50]) by sourceware.org (Postfix) with ESMTPS id D8BE23858D29 for ; Mon, 22 Mar 2021 04:29:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D8BE23858D29 X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 6F192546FAF for ; Mon, 22 Mar 2021 04:28:58 +0000 (UTC) Received: from pdx1-sub0-mail-a69.g.dreamhost.com (100-96-11-34.trex.outbound.svc.cluster.local [100.96.11.34]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 2EF26546F6B for ; Mon, 22 Mar 2021 04:28:56 +0000 (UTC) X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from pdx1-sub0-mail-a69.g.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384) by 100.96.11.34 (trex/6.1.1); Mon, 22 Mar 2021 04:28:58 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Left-Irritate: 1dbfb1396ea5a790_1616387338247_3686461776 X-MC-Loop-Signature: 1616387338247:2988064032 X-MC-Ingress-Time: 1616387338247 Received: from pdx1-sub0-mail-a69.g.dreamhost.com (localhost [127.0.0.1]) by pdx1-sub0-mail-a69.g.dreamhost.com (Postfix) with ESMTP id EB93E8A794 for ; Sun, 21 Mar 2021 21:28:55 -0700 (PDT) Received: from [192.168.1.111] (unknown [1.186.101.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a69.g.dreamhost.com (Postfix) with ESMTPSA id 096627F5EC for ; Sun, 21 Mar 2021 21:28:54 -0700 (PDT) Subject: [PING][PATCH] tunables: Fix comparison of tunable values To: libc-alpha@sourceware.org References: <20210316133429.1382048-1-siddhesh@sourceware.org> X-DH-BACKEND: pdx1-sub0-mail-a69 From: Siddhesh Poyarekar Message-ID: Date: Mon, 22 Mar 2021 09:58:50 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210316133429.1382048-1-siddhesh@sourceware.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3494.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Mar 2021 04:29:31 -0000 On 3/16/21 7:04 PM, Siddhesh Poyarekar via Libc-alpha wrote: > The simplification of tunable_set interfaces took care of > signed/unsigned conversions while setting values, but comparison with > bounds ended up being incorrect; comparing TUNABLE_SIZE_T values for > example will fail because SIZE_MAX is seen as -1. > > Add comparison helpers that take tunable types into account and use > them to do comparison instead. > --- > elf/dl-tunable-types.h | 17 +++++++++++++++++ > elf/dl-tunables.c | 25 ++++++++++++++----------- > elf/dl-tunables.h | 18 ++++++++++++++++++ > 3 files changed, 49 insertions(+), 11 deletions(-) > > diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h > index 626ca334be..39bf738d93 100644 > --- a/elf/dl-tunable-types.h > +++ b/elf/dl-tunable-types.h > @@ -81,4 +81,21 @@ struct _tunable > > typedef struct _tunable tunable_t; > > +static __always_inline bool > +unsigned_tunable_type (tunable_type_code_t t) > +{ > + switch (t) > + { > + case TUNABLE_TYPE_INT_32: > + return false; > + case TUNABLE_TYPE_UINT_64: > + case TUNABLE_TYPE_SIZE_T: > + return true; > + case TUNABLE_TYPE_STRING: > + default: > + break; > + } > + __builtin_unreachable (); > +} > + > #endif > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 1aedb9bd36..8009e54ee5 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -107,32 +107,35 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp, > return; > } > > + 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; > > /* We allow only increasingly restrictive bounds. */ > - if (min < cur->type.min) > + if (tunable_val_lt (min, cur->type.min, unsigned_cmp)) > min = cur->type.min; > > - if (max > cur->type.max) > + if (tunable_val_gt (max, cur->type.max, unsigned_cmp)) > max = cur->type.max; > > /* Skip both bounds if they're inconsistent. */ > - if (min > max) > + if (tunable_val_gt (min, max, unsigned_cmp)) > { > min = cur->type.min; > max = cur->type.max; > } > > - /* Write everything out if the value and the bounds are valid. */ > - if (min <= val && val <= max) > - { > - cur->val.numval = val; > - cur->type.min = min; > - cur->type.max = max; > - cur->initialized = true; > - } > + /* Bail out if the bounds are not valid. */ > + if (tunable_val_lt (val, min, unsigned_cmp) > + || tunable_val_lt (max, val, unsigned_cmp)) > + return; > + > + cur->val.numval = val; > + cur->type.min = min; > + cur->type.max = max; > + cur->initialized = true; > } > > /* Validate range of the input value and initialize the tunable CUR if it looks > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > index ba7ae6b52e..3880e4aab6 100644 > --- a/elf/dl-tunables.h > +++ b/elf/dl-tunables.h > @@ -115,6 +115,24 @@ rtld_hidden_proto (__tunable_set_val) > /* The default value for TUNABLES_FRONTEND. */ > # define TUNABLES_FRONTEND_yes TUNABLES_FRONTEND_valstring > > +static __always_inline bool > +tunable_val_lt (tunable_num_t lhs, tunable_num_t rhs, bool unsigned_cmp) > +{ > + if (unsigned_cmp) > + return (uintmax_t) lhs < (uintmax_t) rhs; > + else > + return lhs < rhs; > +} > + > +static __always_inline bool > +tunable_val_gt (tunable_num_t lhs, tunable_num_t rhs, bool unsigned_cmp) > +{ > + if (unsigned_cmp) > + return (uintmax_t) lhs > (uintmax_t) rhs; > + else > + return lhs > rhs; > +} > + > /* Compare two name strings, bounded by the name hardcoded in glibc. */ > static __always_inline bool > tunable_is_name (const char *orig, const char *envname) >