From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x92f.google.com (mail-ua1-x92f.google.com [IPv6:2607:f8b0:4864:20::92f]) by sourceware.org (Postfix) with ESMTPS id 043403858400 for ; Wed, 10 Nov 2021 18:58:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 043403858400 Received: by mail-ua1-x92f.google.com with SMTP id t13so6865334uad.9 for ; Wed, 10 Nov 2021 10:58:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=J+T3U8nntkR0s7dXp7W1GNZ1nmMhRn8OZ7MDHw+T2N8=; b=cuf/FbEIRGeGIsBY1VK8nCq+EWC5iKCvm10Gri8oRgs+4aTGrtjeJ0qEAsVMaLC1nI Qhi0WMPKlXdT05wbQ6FMsL635ZAbTUMYCvEUcQoKqGty2tgo+kHAiEeS/+ZUpjG0/NQp vQoQ4D70cQh13BdmAkQtR2k9h4Q5hEqI8qEUQSmPEnmBQgBmws+ATQkPApEFhDG6V+jz YUnXbcbMNkW69lX5ZPaXzzEu0bfzMueGMycxsYlywv2YPvdZP4EtZFmgib+/012V3vJm ipFkjGZr2gLXqyv0e6LBsHDoPOaACgzPB5qyW9AHcDSB+hLzS2GMxiQV+DG2EBR7GIvN 6ErQ== X-Gm-Message-State: AOAM532i0fHCK200nRzyuNO3PXK4PoHG8AyAI5WY7Rl6JhdAvWPwy7Cb T73Wdq9C50MJbt1PYNXKlx2/LPJF5vCkKQ== X-Google-Smtp-Source: ABdhPJwMM2tVZJisbJvso5ja7rweQL+h/LK0ad9lPaocfsmSvs6k5LAR39x5IdqZ/iJfK/LhWkc66Q== X-Received: by 2002:a05:6102:3f0d:: with SMTP id k13mr1386960vsv.48.1636570717405; Wed, 10 Nov 2021 10:58:37 -0800 (PST) Received: from birita.. ([2804:431:c7cb:55a:73fa:8bad:ab14:14a3]) by smtp.gmail.com with ESMTPSA id r20sm518149vkq.15.2021.11.10.10.58.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Nov 2021 10:58:36 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: leonardo.macchia@gmail.com Subject: [PATCH 1/3] inet: Fix getnameinfo (NI_NOFQDN) race condition (BZ#28566) Date: Wed, 10 Nov 2021 15:58:30 -0300 Message-Id: <20211110185832.1931688-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211110185832.1931688-1-adhemerval.zanella@linaro.org> References: <20211110185832.1931688-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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: Wed, 10 Nov 2021 18:58:39 -0000 The 'not_first' is accessed on nrl_domainname() in a non atomically way, although it is only updated after the lock is taken. Instead of being too clever, just always take the lock (another option might to use an atomic load and only take the lock if lock might be taken, but I think it would be better to have this optimization on generic lll_lock instead of ad-hoc solution). Checked on x86_64-linux-gnu. --- inet/getnameinfo.c | 120 ++++++++++++++++++++++----------------------- 1 file changed, 59 insertions(+), 61 deletions(-) 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) { - static int not_first; + __libc_lock_define_initialized (static, lock); + __libc_lock_lock (lock); + static bool not_first = false; if (! not_first) { - __libc_lock_define_initialized (static, lock); - __libc_lock_lock (lock); - - if (! not_first) - { - char *c; - struct hostent *h, th; - int herror; - struct scratch_buffer tmpbuf; + char *c; + struct hostent *h, th; + int herror; + struct scratch_buffer tmpbuf; - scratch_buffer_init (&tmpbuf); - not_first = 1; + scratch_buffer_init (&tmpbuf); - while (__gethostbyname_r ("localhost", &th, - tmpbuf.data, tmpbuf.length, - &h, &herror)) + while (__gethostbyname_r ("localhost", &th, + tmpbuf.data, tmpbuf.length, + &h, &herror)) + { + if (herror == NETDB_INTERNAL && errno == ERANGE) { - if (herror == NETDB_INTERNAL && errno == ERANGE) - { - if (!scratch_buffer_grow (&tmpbuf)) - goto done; - } - else - break; + if (!scratch_buffer_grow (&tmpbuf)) + goto done; } + else + break; + } - if (h && (c = strchr (h->h_name, '.'))) + if (h && (c = strchr (h->h_name, '.'))) + domain = __strdup (++c); + else + { + /* The name contains no domain information. Use the name + now to get more information. */ + while (__gethostname (tmpbuf.data, tmpbuf.length)) + if (!scratch_buffer_grow (&tmpbuf)) + goto done; + + if ((c = strchr (tmpbuf.data, '.'))) domain = __strdup (++c); else { - /* The name contains no domain information. Use the name - now to get more information. */ - while (__gethostname (tmpbuf.data, tmpbuf.length)) - if (!scratch_buffer_grow (&tmpbuf)) - goto done; + /* We need to preserve the hostname. */ + const char *hstname = strdupa (tmpbuf.data); + while (__gethostbyname_r (hstname, &th, + tmpbuf.data, + tmpbuf.length, + &h, &herror)) + { + if (herror == NETDB_INTERNAL && errno == ERANGE) + { + if (!scratch_buffer_grow_preserve (&tmpbuf)) + goto done; + } + else + break; + } - if ((c = strchr (tmpbuf.data, '.'))) + if (h && (c = strchr(h->h_name, '.'))) domain = __strdup (++c); else { - /* We need to preserve the hostname. */ - const char *hstname = strdupa (tmpbuf.data); + struct in_addr in_addr; + + in_addr.s_addr = htonl (INADDR_LOOPBACK); - while (__gethostbyname_r (hstname, &th, - tmpbuf.data, tmpbuf.length, + while (__gethostbyaddr_r ((const char *) &in_addr, + sizeof (struct in_addr), + AF_INET, &th, + tmpbuf.data, + tmpbuf.length, &h, &herror)) { if (herror == NETDB_INTERNAL && errno == ERANGE) @@ -146,41 +166,19 @@ nrl_domainname (void) break; } - if (h && (c = strchr(h->h_name, '.'))) + if (h && (c = strchr (h->h_name, '.'))) domain = __strdup (++c); - else - { - struct in_addr in_addr; - - in_addr.s_addr = htonl (INADDR_LOOPBACK); - - while (__gethostbyaddr_r ((const char *) &in_addr, - sizeof (struct in_addr), - AF_INET, &th, - tmpbuf.data, tmpbuf.length, - &h, &herror)) - { - if (herror == NETDB_INTERNAL && errno == ERANGE) - { - if (!scratch_buffer_grow (&tmpbuf)) - goto done; - } - else - break; - } - - if (h && (c = strchr (h->h_name, '.'))) - domain = __strdup (++c); - } } } - done: - scratch_buffer_free (&tmpbuf); } - __libc_lock_unlock (lock); + done: + scratch_buffer_free (&tmpbuf); + not_first = true; } + __libc_lock_unlock (lock); + return domain; }; -- 2.32.0