From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com [IPv6:2607:f8b0:4864:20::f2a]) by sourceware.org (Postfix) with ESMTPS id 0C82B3858436 for ; Tue, 28 Sep 2021 14:38:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C82B3858436 Received: by mail-qv1-xf2a.google.com with SMTP id r18so13537336qvy.8 for ; Tue, 28 Sep 2021 07:38:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=liHtsiG7tAzN1QFbptDX17jKpUfXXHgxi5nTEkGfXdg=; b=gEsSeL4ngLXucYI0hVCnmuZzHg/2L3qnNlENXKr+hPHJz3E1AH1vnS5/NmwU/1Ai/t 1ZO5inZfVmIo2wQ0WUFPwi3hEuD6OpU2XrjIGf1TXCpuD77thl0PYoAYjCgRthynt373 uZe8mv8g0zM7WmBqvt73PDVvnFeDz/2JvIxkqcDecQOI6fR0E/KLB3tMw7dj6ocWIdLZ WXLV0I1ONFk2WKiKNOYVbSYZX9HHvaOQuUX3ahebvvlENPtQImGWavQ1qv/b+j+dxXWR wkbByO2+u7JH1C8zux03Jjuv+CxGVZ7k8Kl0RMn+AX/WaaXQIOozNEDTFEW4ZUjJNiZI q5xg== X-Gm-Message-State: AOAM531tgmnJ3OnUzUzd0JsSpGazbI0x9454OW3JnhoJ0zV76X/DN6fR 4hBUzmaaOO6KsEKdh5Ds9Gsh0lNpMUiMDw== X-Google-Smtp-Source: ABdhPJx4TCa1iGUnbyPzEbZVsdio5NQLv07wtN98vt1sbggOvmBX8Fhau7w5f1wEC74JhUj/ykZ5eA== X-Received: by 2002:a0c:ee8e:: with SMTP id u14mr6054319qvr.11.1632839905322; Tue, 28 Sep 2021 07:38:25 -0700 (PDT) Received: from ?IPv6:2804:431:c7cb:b338:e093:b762:10f5:43e5? ([2804:431:c7cb:b338:e093:b762:10f5:43e5]) by smtp.gmail.com with ESMTPSA id z6sm2322567qkl.31.2021.09.28.07.38.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Sep 2021 07:38:24 -0700 (PDT) Subject: Re: [PATCH v3] Linux: Simplify __opensock and fix race condition [BZ #28353] To: Florian Weimer , libc-alpha@sourceware.org References: <87wnn1df57.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Message-ID: <15c84fc0-27fa-c9aa-16ca-e50b647718fc@linaro.org> Date: Tue, 28 Sep 2021 11:38:23 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <87wnn1df57.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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: Tue, 28 Sep 2021 14:38:28 -0000 On 27/09/2021 15:53, Florian Weimer via Libc-alpha wrote: > AF_NETLINK support is not quite optional on modern Linux systems > anymore, so it is likely that the first attempt will always succeed. > Consequently, there is no need to cache the result. Keep AF_UNIX > and the Internet address families as a fallback, for the rare case > that AF_NETLINK is missing. The other address families previously > probed are totally obsolete be now, so remove them. > > Use this simplified version as the generic implementation, disabling > Netlink support as needed. > > --- > v2: Replace generic version. > v3: Fix typo in commit message. LGTM, thanks. Reviewed-by: Adhemerval Zanella > > socket/opensock.c | 63 ++++++------------ > sysdeps/unix/sysv/linux/opensock.c | 114 -------------------------------- > sysdeps/unix/sysv/linux/s390/opensock.c | 2 - > 3 files changed, 21 insertions(+), 158 deletions(-) > > diff --git a/socket/opensock.c b/socket/opensock.c > index 37148d4743..ff94d27a61 100644 > --- a/socket/opensock.c > +++ b/socket/opensock.c > @@ -1,4 +1,5 @@ > -/* Copyright (C) 1999-2021 Free Software Foundation, Inc. > +/* Create socket with an unspecified address family for use with ioctl. > + Copyright (C) 1999-2021 Free Software Foundation, Inc. > This file is part of the GNU C Library. > > The GNU C Library is free software; you can redistribute it and/or Ok. > @@ -15,56 +16,34 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > +#include > #include > -#include > > /* Return a socket of any type. The socket can be used in subsequent > ioctl calls to talk to the kernel. */ > int > __opensock (void) > { > - /* Cache the last AF that worked, to avoid many redundant calls to > - socket(). */ > - static int sock_af = -1; > - int fd = -1; > - __libc_lock_define_initialized (static, lock); > + /* SOCK_DGRAM is supported by all address families. (Netlink does > + not support SOCK_STREAM.) */ > + int type = SOCK_DGRAM | SOCK_CLOEXEC; > + int fd; > > - if (sock_af != -1) > - { > - fd = __socket (sock_af, SOCK_DGRAM, 0); > - if (fd != -1) > - return fd; > - } > - > - __libc_lock_lock (lock); > - > - if (sock_af != -1) > - fd = __socket (sock_af, SOCK_DGRAM, 0); > - > - if (fd == -1) > - { > -#ifdef AF_INET > - fd = __socket (sock_af = AF_INET, SOCK_DGRAM, 0); > -#endif > -#ifdef AF_INET6 > - if (fd < 0) > - fd = __socket (sock_af = AF_INET6, SOCK_DGRAM, 0); > -#endif > -#ifdef AF_IPX > - if (fd < 0) > - fd = __socket (sock_af = AF_IPX, SOCK_DGRAM, 0); > -#endif > -#ifdef AF_AX25 > - if (fd < 0) > - fd = __socket (sock_af = AF_AX25, SOCK_DGRAM, 0); > -#endif > -#ifdef AF_APPLETALK > - if (fd < 0) > - fd = __socket (sock_af = AF_APPLETALK, SOCK_DGRAM, 0); > +#ifdef AF_NETLINK > + fd = __socket (AF_NETLINK, type, 0); > + if (fd >= 0) > + return fd; > #endif > - } > > - __libc_lock_unlock (lock); > + fd = __socket (AF_UNIX, type, 0); > + if (fd >= 0) > + return fd; > + fd = __socket (AF_INET, type, 0); > + if (fd >= 0) > + return fd; > + fd = __socket (AF_INET6, type, 0); > + if (fd >= 0) > + return fd; > + __set_errno (ENOENT); > return fd; > } Ok. > diff --git a/sysdeps/unix/sysv/linux/opensock.c b/sysdeps/unix/sysv/linux/opensock.c > deleted file mode 100644 > index e87d6e58b0..0000000000 > --- a/sysdeps/unix/sysv/linux/opensock.c > +++ /dev/null > @@ -1,114 +0,0 @@ > -/* Copyright (C) 1999-2021 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library; if not, see > - . */ > - > -#include > -#include > -#include > -#include > -#include > -#include > - > -/* Return a socket of any type. The socket can be used in subsequent > - ioctl calls to talk to the kernel. */ > -int > -__opensock (void) > -{ > - static int last_family; /* Available socket family we will use. */ > - static int last_type; > - static const struct > - { > - int family; > - const char procname[15]; > - } afs[] = > - { > - { AF_UNIX, "net/unix" }, > - { AF_INET, "" }, > - { AF_INET6, "net/if_inet6" }, > - { AF_AX25, "net/ax25" }, > - { AF_NETROM, "net/nr" }, > - { AF_ROSE, "net/rose" }, > - { AF_IPX, "net/ipx" }, > - { AF_APPLETALK, "net/appletalk" }, > - { AF_ECONET, "sys/net/econet" }, > - { AF_ASH, "sys/net/ash" }, > - { AF_X25, "net/x25" }, > -#ifdef NEED_AF_IUCV > - { AF_IUCV, "net/iucv" } > -#endif > - }; > -#define nafs (sizeof (afs) / sizeof (afs[0])) > - char fname[sizeof "/proc/" + 14]; > - int result; > - int has_proc; > - size_t cnt; > - > - /* We already know which family to use from the last call. Use it > - again. */ > - if (last_family != 0) > - { > - assert (last_type != 0); > - > - result = __socket (last_family, last_type | SOCK_CLOEXEC, 0); > - if (result != -1 || errno != EAFNOSUPPORT) > - /* Maybe the socket type isn't supported anymore (module is > - unloaded). In this case again try to find the type. */ > - return result; > - > - /* Reset the values. They seem not valid anymore. */ > - last_family = 0; > - last_type = 0; > - } > - > - /* Check whether the /proc filesystem is available. */ > - has_proc = __access ("/proc/net", R_OK) != -1; > - strcpy (fname, "/proc/"); > - > - /* Iterate over the interface families and find one which is > - available. */ > - for (cnt = 0; cnt < nafs; ++cnt) > - { > - int type = SOCK_DGRAM; > - > - if (has_proc && afs[cnt].procname[0] != '\0') > - { > - strcpy (fname + 6, afs[cnt].procname); > - if (__access (fname, R_OK) == -1) > - /* The /proc entry is not available. I.e., we cannot > - create a socket of this type (without loading the > - module). Don't look for it since this might trigger > - loading the module. */ > - continue; > - } > - > - if (afs[cnt].family == AF_NETROM || afs[cnt].family == AF_X25) > - type = SOCK_SEQPACKET; > - > - result = __socket (afs[cnt].family, type | SOCK_CLOEXEC, 0); > - if (result != -1) > - { > - /* Found an available family. */ > - last_type = type; > - last_family = afs[cnt].family; > - return result; > - } > - } > - > - /* None of the protocol families is available. It is unclear what kind > - of error is returned. ENOENT seems like a reasonable choice. */ > - __set_errno (ENOENT); > - return -1; > -} Ok. > diff --git a/sysdeps/unix/sysv/linux/s390/opensock.c b/sysdeps/unix/sysv/linux/s390/opensock.c > deleted file mode 100644 > index f099d651ff..0000000000 > --- a/sysdeps/unix/sysv/linux/s390/opensock.c > +++ /dev/null > @@ -1,2 +0,0 @@ > -#define NEED_AF_IUCV 1 > -#include "../opensock.c" > Ok.