From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-5.6 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_HI,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 92E821F8C6 for ; Thu, 26 Aug 2021 17:07:54 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CF5C6385843B for ; Thu, 26 Aug 2021 17:07:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CF5C6385843B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629997673; bh=liZeyCjqmpF2GvsloICIispDDzJcqzUohCWtkfNC4NA=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=rcKdhypLh6m5Vj4QJRPIqpvL+11Q9AqnFyGGpz9QFdriD+ZwvUUNegWBzYOlRizNi lEDVodKGGIMMydFAfjmff7hcwsiwDKWfrP5pWPFLZyb7p/OZawDKi3UXYSNz0oNCEM OWch7O3h4evsnIIe32qUdUjQ9C6dsRVfrKqm/+zg= Received: from mail-qt1-x833.google.com (mail-qt1-x833.google.com [IPv6:2607:f8b0:4864:20::833]) by sourceware.org (Postfix) with ESMTPS id 8D2E63858001 for ; Thu, 26 Aug 2021 17:07:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8D2E63858001 Received: by mail-qt1-x833.google.com with SMTP id r21so3042217qtw.11 for ; Thu, 26 Aug 2021 10:07:02 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=liZeyCjqmpF2GvsloICIispDDzJcqzUohCWtkfNC4NA=; b=pZtvzb3sxcdQtX8YmE6C2pMBpdOszLz26Wp8qzVxOaLHz/rBAgA/vaYX3NbPUGC/Un HgZaZh9Dc5GHkBSJcebPRLacCohIeNTGCOCg4D+8aeO07vKCSHi5AOXAcy03MMjyQoJ0 W6K28sSBjN22FOgdxvYU5rhF5NqaX3SwxYChT9BHlF+UJBdYwyt7r2bXPmEgmHzbmgbq 91lbNtjksPsNTHFnbeXBugygqi8TKvIvoVl1tQ97K4NvSDZnNJO7KG61nWPyx45iPc33 UiV31WDY4qO64be8vJlz1wxkX8ykk4IINMIYaCtWK6w7VCs88k/h43RJtzrblcxY3cFJ bXOg== X-Gm-Message-State: AOAM533mEV4l2jm536KQDIDqY/ASEbmpke3LRHBbAUyBameglIEUL7ZM oSSMprG/dGlKT0LwQJFljlf+1ppwQO3FgA== X-Google-Smtp-Source: ABdhPJwnMl0rpt3l/u1x6KGBJ+hSg9XYcGVDIgG4Z/GSt5GI5QPrnXkSN+1x6NgPnPR7RHHwpJe/9A== X-Received: by 2002:ac8:6b82:: with SMTP id z2mr4270329qts.52.1629997621913; Thu, 26 Aug 2021 10:07:01 -0700 (PDT) Received: from ?IPv6:2804:431:c7ca:1a68:4e82:9e7c:b7e4:7f3? ([2804:431:c7ca:1a68:4e82:9e7c:b7e4:7f3]) by smtp.gmail.com with ESMTPSA id c15sm2880477qka.46.2021.08.26.10.07.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 26 Aug 2021 10:07:01 -0700 (PDT) Subject: Re: [PATCH v2 09/19] nptl: Fix race between pthread_kill and thread exit (bug 12889) To: Florian Weimer References: <20210823195047.543237-1-adhemerval.zanella@linaro.org> <20210823195047.543237-10-adhemerval.zanella@linaro.org> <878s0o70ab.fsf@oldenburg.str.redhat.com> Message-ID: Date: Thu, 26 Aug 2021 14:06:59 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <878s0o70ab.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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: , From: Adhemerval Zanella via Libc-alpha Reply-To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 26/08/2021 11:23, Florian Weimer wrote: > * Adhemerval Zanella: > >> diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c >> index 5d4c86f920..63fe8c44ca 100644 >> --- a/nptl/pthread_kill.c >> +++ b/nptl/pthread_kill.c >> @@ -26,15 +26,18 @@ __pthread_kill_internal (pthread_t threadid, int signo) >> pid_t tid; >> struct pthread *pd = (struct pthread *) threadid; >> >> + /* Block all signal, since the lock is recursive and used on pthread_cancel >> + (which should be async-signal-safe). */ >> + sigset_t oldmask; >> + __libc_signal_block_all (&oldmask); >> + lll_lock (pd->tidlock, LLL_PRIVATE); >> + >> if (pd == THREAD_SELF) >> /* It is a special case to handle raise() implementation after a vfork >> call (which does not update the PD tid field). */ >> tid = INLINE_SYSCALL_CALL (gettid); >> else >> - /* Force load of pd->tid into local variable or register. Otherwise >> - if a thread exits between ESRCH test and tgkill, we might return >> - EINVAL, because pd->tid would be cleared by the kernel. */ >> - tid = atomic_forced_read (pd->tid); >> + tid = pd->tid; >> >> int val; >> if (__glibc_likely (tid > 0)) >> @@ -53,6 +56,9 @@ __pthread_kill_internal (pthread_t threadid, int signo) >> as an error. */ >> val = 0; >> >> + lll_unlock (pd->tidlock, LLL_PRIVATE); >> + __libc_signal_restore_set (&oldmask); >> + >> return val; >> } > > This needs a comment explaining that *all* pending signals are delivered > at the point of the __libc_signal_restore_set call. I hope that this is > actually what happens in Linux; POSIX only guarantees that for one > pending signal that is unblocked. My understanding from kernel/signal.c all pending signals are delivered with the signal mask restore, but only real-time one are queued (and subjected to RLIMIT_SIGPENDING, which causes another issue [1]). > > The problem here is that pthread_kill (pthread_self (), …) must generate > synchronous signal, and due to the signal-blocking, it is not > immediately obvious if that's the case. > > If we aren't sure that all signals are flushed, we need to check for the > send-signal-to-self case and avoid blocking the signal there. We don't > need the tidlock for that because the running thread won't go away. > > Thanks, > Florian > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=21108