From patchwork Wed Nov 10 18:58:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 47425 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 373853857C73 for ; Wed, 10 Nov 2021 18:59:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 373853857C73 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1636570780; bh=J+T3U8nntkR0s7dXp7W1GNZ1nmMhRn8OZ7MDHw+T2N8=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=bTacc2jLabtxmCVsd+I2BR6PdIbC8X6IFMDA9wZrt1mULo07r7sdBjrpqqbg5haUU jns0Uq4jtKmoBQ4nuUpA1/NWn7+Fc9cvGC8E8HdwluDriQS+zXa2JwqqosN0ewQJ70 odHE2wrl+m1dviIUkYzTR6pz25QG8dYFAdQWn5cw= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org 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) To: libc-alpha@sourceware.org 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 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Netto Reply-To: Adhemerval Zanella Cc: leonardo.macchia@gmail.com Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" 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; };