From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf30.google.com (mail-qv1-xf30.google.com [IPv6:2607:f8b0:4864:20::f30]) by sourceware.org (Postfix) with ESMTPS id 201233857819 for ; Tue, 6 Apr 2021 19:40:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 201233857819 Received: by mail-qv1-xf30.google.com with SMTP id e19so1375395qvu.3 for ; Tue, 06 Apr 2021 12:40:11 -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=llFMnmaMHptU/7kL2SFMyCDM9EbT692X7ZLpXCqJ2yc=; b=GMjIZR1lDKB+V2QX8vlSj+XuzMpGIA5LHx4ET0idsS2OHW5fEeGsh8owdJkNxp+3gG dzfJNpj2QXBk7mqvwzS4Jo76yx4Wi5HkvwMXeHmgulcRu4eYCrpKIvKGf5A1xksuwkSO lMNMcKyD3rIp82cEx2uhma7J4XNHPQg2OXZor0W0LbZNxsIkNtSq8dz/bpzAaODhW7Iv 5cg67SXtJ6VDjfhzb3Nvk1MsZzIvUvlss20iJ7nnDvFatCtQyxQVcZEYwHYKn1nJwmG7 7gVshR0XXAOyEHw9PKTlvaj59JrM1cMYex+YJIJUpXXOEfLaW7zjFXo+hzCUCN/BQ/sB fI0g== X-Gm-Message-State: AOAM532U7VA915RJ7x+IvIj4Lpi52ZlRMKMWg+CMXRVdZKRYrTr1aWV1 8MVV1yaVniPvI4qXwcRRc6NFN9ZEkbOH/49F X-Google-Smtp-Source: ABdhPJx9wPW2YyJAtYA9pzCaEERM3IQxlxu3JblV+UuZDNQjqYWQeLIITP7LvYeIfCEObDWdISr/ew== X-Received: by 2002:a0c:80ca:: with SMTP id 68mr30491692qvb.12.1617738010404; Tue, 06 Apr 2021 12:40:10 -0700 (PDT) Received: from [192.168.1.132] ([177.194.41.149]) by smtp.gmail.com with ESMTPSA id y9sm16250808qkm.19.2021.04.06.12.40.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 06 Apr 2021 12:40:10 -0700 (PDT) Subject: Re: [PATCH 07/15] elf: Refactor _dl_update_slotinfo to avoid use after free To: Szabolcs Nagy , libc-alpha@sourceware.org References: <3ecdb956cbf6d1b46e36311ffe7f491ce186cdbc.1613390045.git.szabolcs.nagy@arm.com> From: Adhemerval Zanella Message-ID: <2f925732-4c85-4dfe-036e-ed2dde651202@linaro.org> Date: Tue, 6 Apr 2021 16:40:07 -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: <3ecdb956cbf6d1b46e36311ffe7f491ce186cdbc.1613390045.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: Tue, 06 Apr 2021 19:40:12 -0000 On 15/02/2021 09:00, Szabolcs Nagy via Libc-alpha wrote: > map is not valid to access here because it can be freed by a > concurrent dlclose, so don't check the modid. Won't it be protected by the recursive GL(dl_load_lock) in such case? I think the concurrency issue is between dlopen and _dl_deallocate_tls called by pthread stack handling (nptl/allocatestack.c). Am I missing something here? > > 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). > --- > 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); > >