From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x831.google.com (mail-qt1-x831.google.com [IPv6:2607:f8b0:4864:20::831]) by sourceware.org (Postfix) with ESMTPS id 100D93989CB7 for ; Wed, 14 Apr 2021 18:20:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 100D93989CB7 Received: by mail-qt1-x831.google.com with SMTP id i6so6555473qti.10 for ; Wed, 14 Apr 2021 11:20:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=BZJ3Vx1QupKm+utED1YN+jRVzvY5gf/bo/HNlYh/aSs=; b=o4raJ/WfBpNiwAZT+CmlEV8aXHd/uUt26X1/t0PFFnsKf+fcI7WrhVOBP0P+DfPLE2 xglGb3YKJjYybJqphYDhdOceMOEOmvUeR4f/h00MvQ9/ulVYyacAvEpqpFIVPKmS0r5z COvafIa87r/Rx9itGG47+4sDBv0XpOmmcQPR75pgLIjpB+F89k2L0oqkBi3Ngeu5v/qy tgOLSn5oMTAxwV4K1DnZdeYZe4KXXps2BFUlsdPpDVdtYbKZ2N8AlOKmetQZxMHfhCyq 7hoVOQnkIqMLNpSePkdd8UwYQQY0yhBSgDKZzdDKBSSH98+VHbbxgUbyyugQQOCaWgpd nY9Q== X-Gm-Message-State: AOAM532v0/XU27fS2HJ/d1d2tB5Y65LvoTPuw7mo9CW3kfmkn4GPT3uz 9E7ilNf6mMYigwzcQtdrFtUNQ5oqnTV9+0S2 X-Google-Smtp-Source: ABdhPJyAY/nq5Sw8ZhFzAJBZTt1kiDhrA59hPakkPqc4C4X7nNS5A68dwokXcPuxahay/yHk9K5sDw== X-Received: by 2002:ac8:74da:: with SMTP id j26mr13875488qtr.179.1618424443576; Wed, 14 Apr 2021 11:20:43 -0700 (PDT) Received: from [192.168.1.132] ([177.194.41.149]) by smtp.gmail.com with ESMTPSA id f12sm99180qtq.84.2021.04.14.11.20.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 14 Apr 2021 11:20:43 -0700 (PDT) Subject: Re: [PATCH v2 04/14] elf: Refactor _dl_update_slotinfo to avoid use after free To: libc-alpha@sourceware.org, Szabolcs Nagy References: <3237824fb057632817a8de508d1fcb1f2e6f5c7e.1618301209.git.szabolcs.nagy@arm.com> From: Adhemerval Zanella Message-ID: <5f8fd117-3b6a-f5ec-2614-b02878e7b7b6@linaro.org> Date: Wed, 14 Apr 2021 15:20:41 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <3237824fb057632817a8de508d1fcb1f2e6f5c7e.1618301209.git.szabolcs.nagy@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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: Wed, 14 Apr 2021 18:20:45 -0000 On 13/04/2021 05:18, Szabolcs Nagy via Libc-alpha wrote: > map is not valid to access here because it can be freed by a concurrent > dlclose: during tls access (via __tls_get_addr) _dl_update_slotinfo is > called without holding dlopen locks. So don't check the modid of map. > > The map == 0 and map != 0 code paths can be shared (avoiding the dtv > resize in case of map == 0 is just an optimization: larger dtv than > necessary would be fine too). > > -- > v2: > - commit message update LGTM, thanks. Reviewed-by: Adhemerval Zanella > --- > elf/dl-tls.c | 21 +++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) > > diff --git a/elf/dl-tls.c b/elf/dl-tls.c > index 24d00c14ef..f8b32b3ecb 100644 > --- a/elf/dl-tls.c > +++ b/elf/dl-tls.c > @@ -743,6 +743,8 @@ _dl_update_slotinfo (unsigned long int req_modid) > { > for (size_t cnt = total == 0 ? 1 : 0; cnt < listp->len; ++cnt) > { > + size_t modid = total + cnt; > + > size_t gen = listp->slotinfo[cnt].gen; > > if (gen > new_gen) > @@ -758,25 +760,12 @@ _dl_update_slotinfo (unsigned long int req_modid) > > /* If there is no map this means the entry is empty. */ > struct link_map *map = listp->slotinfo[cnt].map; > - if (map == NULL) > - { > - if (dtv[-1].counter >= total + cnt) > - { > - /* If this modid was used at some point the memory > - might still be allocated. */ > - free (dtv[total + cnt].pointer.to_free); > - dtv[total + cnt].pointer.val = TLS_DTV_UNALLOCATED; > - dtv[total + cnt].pointer.to_free = NULL; > - } > - > - continue; > - } > - > /* Check whether the current dtv array is large enough. */ > - size_t modid = map->l_tls_modid; > - assert (total + cnt == modid); > if (dtv[-1].counter < modid) > { > + if (map == NULL) > + continue; > + > /* Resize the dtv. */ > dtv = _dl_resize_dtv (dtv); > >