From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x736.google.com (mail-qk1-x736.google.com [IPv6:2607:f8b0:4864:20::736]) by sourceware.org (Postfix) with ESMTPS id 1305B395A00B for ; Thu, 27 May 2021 17:28:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1305B395A00B Received: by mail-qk1-x736.google.com with SMTP id q10so1425896qkc.5 for ; Thu, 27 May 2021 10:28:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=o2eS4AfUTQYAL1vRALavhsT4l3P6Hdm4Ms1WTR5ehoY=; b=ugVijWDs5xtz3CMJ6jRT2TbyoFfnGio/szw35Xn3PKwHdL8fXyIMS/lmgmtO35DTgj yKcRZ4p06ia9a0+BXrS5vZCduAHHPKWSL9xqFQA+mza1IQ+gDWwyXoyAng2CTjbjRVrC YdVnfl8tAjTFZXhzSfm22oPbpN2c2iB0NortUx2H3jFPi808kohRu2xJUKme8wVerXra IuGU4e1f8r5Yn/0QpzH/fgX1S5jA7UZ6+uBzAq+KzKJ9DyS1qeIApUfJY0m1JATx0S7d dcx0LrUFgotZ6Zpp3Jq+quyKz72IQYz2vic5q8HOGwGQlegr9GRZSE7/Y1hDTCYvh5d3 t/mQ== X-Gm-Message-State: AOAM533S5gcKA7+F6AtFzW5ssTV9sgsDc6Q40eN4yTF9ZsYlyWMou638 h9spFkerzaTjKb7CgDsJzZ/N5KJH6t6PEg== X-Google-Smtp-Source: ABdhPJzSIOhObBso/1Gxvo6Y+6VCOqFC7kO8uF/2sEjj17C2pRUbJnETS0DGVR/yCGC7o6lhDOSGNg== X-Received: by 2002:a05:620a:a48:: with SMTP id j8mr4684628qka.135.1622136519537; Thu, 27 May 2021 10:28:39 -0700 (PDT) Received: from birita.. ([177.194.37.86]) by smtp.googlemail.com with ESMTPSA id 25sm1767913qtd.51.2021.05.27.10.28.38 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 May 2021 10:28:39 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH v2 9/9] nptl: Avoid async cancellation to wrongly update __nptl_nthreads (BZ #19366) Date: Thu, 27 May 2021 14:28:23 -0300 Message-Id: <20210527172823.3461314-10-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> References: <20210527172823.3461314-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.6 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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 27 May 2021 17:28:41 -0000 The testcase provided on BZ#19366 may update __nptl_nthreads in a wrong order, triggering an early process exit because the thread decrement the value twice. The issue is once the thread exits without acting on cancellation, it decreaments '__nptl_nthreads' and then atomically set 'cancelhandling' with EXITING_BIT (thus preventing further cancellation handler to act). The issue happens if a SIGCANCEL is received between checking '__ntpl_nthreads' and setting EXITING_BIT. To avoid it, the '__nptl_nthreads' decrement is moved after EXITING_BIT. It does not fully follow the POSIX XSH 2.9.5 Thread Cancellation under the heading Thread Cancellation Cleanup Handlers that states that when a cancellation request is acted upon, or when a thread calls pthread_exit(), the thread first disables cancellation by setting its cancelability state to PTHREAD_CANCEL_DISABLE and its cancelability type to PTHREAD_CANCEL_DEFERRED. The issue is '__pthread_enable_asynccancel' explicit enabled assynchrnous cancellation, so an interrupted syscall within the cancellation cleanup handlers might see an invalid cancelling type (a possible fix might be possible with my proposed solution to BZ#12683). Trying to come up with a test is quite hard since it requires to mimic the timing issue described below, however I see that the buz report reproducer does not early exit anymore. Checked on x86_64-linux-gnu. --- nptl/pthread_create.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 15ce5ad4a1..79af3412cf 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -442,13 +442,6 @@ start_thread (void *arg) /* Clean up any state libc stored in thread-local variables. */ __libc_thread_freeres (); - /* If this is the last thread we terminate the process now. We - do not notify the debugger, it might just irritate it if there - is no thread left. */ - if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads))) - /* This was the last thread. */ - exit (0); - /* Report the death of the thread if this is wanted. */ if (__glibc_unlikely (pd->report_events)) { @@ -483,6 +476,10 @@ start_thread (void *arg) the breakpoint reports TD_THR_RUN state rather than TD_THR_ZOMBIE. */ atomic_bit_set (&pd->cancelhandling, EXITING_BIT); + if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads))) + /* This was the last thread. */ + exit (0); + #ifndef __ASSUME_SET_ROBUST_LIST /* If this thread has any robust mutexes locked, handle them now. */ # if __PTHREAD_MUTEX_HAVE_PREV -- 2.30.2