From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk1-xa2e.google.com (mail-vk1-xa2e.google.com [IPv6:2607:f8b0:4864:20::a2e]) by sourceware.org (Postfix) with ESMTPS id 13A3F385840A for ; Thu, 11 Nov 2021 13:34:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 13A3F385840A Received: by mail-vk1-xa2e.google.com with SMTP id f7so3259406vkf.10 for ; Thu, 11 Nov 2021 05:34:31 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=SZrAKBh0xDFjd3R8aH+l09qLAeYdf0O4ZT93DfZH0DU=; b=XvZP70WIRRoeXjUi+yxLFeCWAdAx6Mq+HTXEYMzhNH6rnbuIfWPjP9Fb9hU+HU27Zg SEydpMCEA7hKcD3QNAVQQj2Q3Vc2lx9WqWyfxG6hkDbCLhUWtcncksEgmIBKgcCuUGvr AGHgMq6YnIvaB/5+aph3zm8hdVrv+x4YP8IqTRq5BtMLsVh3HvyA+QjA2RsSpIOtn5A8 5c6u1HrY6S0gL7VK2H5UTY8HIJDR6EZE0ABsJPngNs31OaGcAJr7mKnQSn/QzsVi6BY/ ehLh1V30nZ3X3wwkMPSQ5izvSFf/Qle3Tuw+xCtEP80DBxNymebKejLIz3mz6ii3FUOM oYqQ== X-Gm-Message-State: AOAM533hsrEopdUA9s7Z64NOAxa6Lo4J3FVxR17GwHcxggoz/I2sYIyi gK3IlSOiAl325yuh9HNmjQXLqQ== X-Google-Smtp-Source: ABdhPJxNOtTOs6Ox6EALV+8PPYUjRIy7/8MtWLEC8W+YdgC/N/cGhSaAxinjyWjevKVlhY5wNsl63w== X-Received: by 2002:a05:6122:1782:: with SMTP id o2mr11711479vkf.3.1636637670563; Thu, 11 Nov 2021 05:34:30 -0800 (PST) Received: from ?IPV6:2804:431:c7cb:55a:48f2:1d0b:8ae8:643a? ([2804:431:c7cb:55a:48f2:1d0b:8ae8:643a]) by smtp.gmail.com with ESMTPSA id j31sm2006520uag.4.2021.11.11.05.34.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Nov 2021 05:34:30 -0800 (PST) Message-ID: <27605b2b-cde0-9b1a-fc97-882d30e30b49@linaro.org> Date: Thu, 11 Nov 2021 10:34:27 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.1 Subject: Re: [PATCH 1/3] inet: Fix getnameinfo (NI_NOFQDN) race condition (BZ#28566) Content-Language: en-US To: Florian Weimer , Adhemerval Zanella via Libc-alpha Cc: leonardo.macchia@gmail.com References: <20211110185832.1931688-1-adhemerval.zanella@linaro.org> <20211110185832.1931688-2-adhemerval.zanella@linaro.org> <87r1bn150g.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella In-Reply-To: <87r1bn150g.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-15.1 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 11 Nov 2021 13:34:32 -0000 On 11/11/2021 05:16, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> diff --git a/inet/getnameinfo.c b/inet/getnameinfo.c >> index 8380d85783..58ebbb1154 100644 >> --- a/inet/getnameinfo.c >> +++ b/inet/getnameinfo.c >> @@ -86,55 +86,75 @@ libc_freeres_ptr (static char *domain); >> static char * >> nrl_domainname (void) >> { >> + __libc_lock_define_initialized (static, lock); >> + __libc_lock_lock (lock); >> >> + static bool not_first = false; >> if (! not_first) > >> + done: >> + scratch_buffer_free (&tmpbuf); >> + not_first = true; > > This is missing the acquire/release pairing for the double-checked > locking idiom. You can probably use the domain variable directly. But it is done now within the lock, different than current implementation which does outside. I moved to be within the lock exactly to avoid the double-checked locking idiom. I think now that we might be moving to a more optimized lll_lock internally using a acquire-load+CAS instead of just CAS we can get it without need to code it explicitly. > >> - if ((c = strchr (tmpbuf.data, '.'))) >> + if (h && (c = strchr(h->h_name, '.'))) > > h != NULL? I tried to make the code as-is, since is essentially removing a tab; but I think it makes sense. > >> + if (!scratch_buffer_grow_preserve (&tmpbuf)) > > > I think the change to _preserve should be in the alloca elimination > patch (but see my comment there). Yeah, this is in fact something I did saw on my rebase, but I am not sure why it is still in the patch submission.