From patchwork Sat Feb 14 13:40:12 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 130127 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 4DDBF4BAD158 for ; Sat, 14 Feb 2026 13:40:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4DDBF4BAD158 X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from sonata.ens-lyon.org (domu-toccata.ens-lyon.fr [140.77.166.138]) by sourceware.org (Postfix) with ESMTPS id 8BC274BAD144 for ; Sat, 14 Feb 2026 13:40:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8BC274BAD144 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ens-lyon.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=bounce.ens-lyon.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8BC274BAD144 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=140.77.166.138 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1771076418; cv=none; b=TDmzoMHZXczwfrDrdb4m2WTFShnL+41a3lf+ozaU/a0JpA2xBmr6YMIL3HExalTOv0gANbKWwJrfst9unbSHV76IQM0JSV09Fu64Oq57rvvlpcgAIX4PQ3IN5Sj/dcCEFlskJBNfyV4llLXsD4oIlLDW3hcQkx5mupkDwRdaoOk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1771076418; c=relaxed/simple; bh=7prA98AfvR01rnDiWX52+JO9dmJvc/K/j3bT+V9BVAc=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=PTJ1n29pciIU3kdiRy/SBh2A/stcSqlmL1LXlyFq/pgjxqM8mqMeoJ0k0AorVvSI/Joph1EEtjXLqOagXj5iea+f1flBE+TEm5+TFHx0HAdT0BLtVWy8TJ6v4qdqzLPNCzGvGwjI8NHGPptGxryL3xxG9821DyndGei1P/ilFmo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8BC274BAD144 Received: from localhost (localhost [127.0.0.1]) by sonata.ens-lyon.org (Postfix) with ESMTP id 43688A03F3; Sat, 14 Feb 2026 14:40:17 +0100 (CET) Received: from sonata.ens-lyon.org ([127.0.0.1]) by localhost (sonata.ens-lyon.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uygHnz8T6FRk; Sat, 14 Feb 2026 14:40:17 +0100 (CET) Received: from end (unknown [212.133.41.37]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by sonata.ens-lyon.org (Postfix) with ESMTPSA id 916A8A023D; Sat, 14 Feb 2026 14:40:16 +0100 (CET) Received: from samy by end with local (Exim 4.99.1) (envelope-from ) id 1vrFsb-00000005xu3-3TDZ; Sat, 14 Feb 2026 14:40:13 +0100 From: Samuel Thibault To: libc-alpha@sourceware.org Cc: Mike Kelly , commit-hurd@gnu.org, Samuel Thibault Subject: [hurd, commited] hurd: calling alarm() whilst handling SIGALRM can deadlock. Date: Sat, 14 Feb 2026 14:40:12 +0100 Message-ID: <20260214134013.1421875-1-samuel.thibault@ens-lyon.org> X-Mailer: git-send-email 2.51.0 MIME-Version: 1.0 X-Spam-Status: No, score=-13.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_HELO_PASS, SPF_PASS, TXREP, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~patchwork=sourceware.org@sourceware.org From: Mike Kelly alarm() and restart_itimer() can attempt to acquire _hurd_siglock and _hurd_itimer_lock in opposite sequence resulting in occasional deadlock. Rearranged to always acquire the locks in the same sequence with a new pre-condition that setitimer_locked() must be called with both locks already acquired. Message-ID: <20260214091715.157471-2-mike@weatherwax.co.uk> Reviewed-by: Samuel Thibault --- sysdeps/mach/hurd/setitimer.c | 42 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/sysdeps/mach/hurd/setitimer.c b/sysdeps/mach/hurd/setitimer.c index 5a57280e2c..265576e00e 100644 --- a/sysdeps/mach/hurd/setitimer.c +++ b/sysdeps/mach/hurd/setitimer.c @@ -131,8 +131,7 @@ timer_thread (void) /* Forward declaration. */ static int setitimer_locked (const struct itimerval *new, - struct itimerval *old, void *crit, - int hurd_siglocked); + struct itimerval *old); static sighandler_t restart_itimer (struct hurd_signal_preemptor *preemptor, @@ -144,21 +143,25 @@ restart_itimer (struct hurd_signal_preemptor *preemptor, struct itimerval it; /* Either reload or disable the itimer. */ + /* _hurd_siglock is already locked by the caller. */ __spin_lock (&_hurd_itimer_lock); it.it_value = it.it_interval = _hurd_itimerval.it_interval; - setitimer_locked (&it, NULL, NULL, 1); + setitimer_locked (&it, NULL); + __spin_unlock (&_hurd_itimer_lock); /* Continue with normal delivery (or hold, etc.) of SIGALRM. */ return SIG_ERR; } -/* Called before any normal SIGALRM signal is delivered. - Reload the itimer, or disable the itimer. */ +/* Called before any normal SIGALRM signal is delivered. Reload the + itimer, or disable the itimer. _hurd_siglock and _hurd_itimer_lock + must be locked before entry noting that it is important to acquire + the _hurd_siglock before the _hurd_itimer_lock. Both of these + remain locked on exit. */ static int -setitimer_locked (const struct itimerval *new, struct itimerval *old, - void *crit, int hurd_siglocked) +setitimer_locked (const struct itimerval *new, struct itimerval *old) { struct itimerval newval; struct timeval now, remaining, elapsed; @@ -180,8 +183,6 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old, This is what BSD does, even though it's not documented. */ if (old) *old = _hurd_itimerval; - spin_unlock (&_hurd_itimer_lock); - _hurd_critical_section_unlock (crit); return 0; } @@ -199,16 +200,12 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old, __sigmask (SIGALRM), SI_TIMER, SI_TIMER, &restart_itimer, }; - if (!hurd_siglocked) - __mutex_lock (&_hurd_siglock); if (! preemptor.next && _hurdsig_preemptors != &preemptor) { preemptor.next = _hurdsig_preemptors; _hurdsig_preemptors = &preemptor; _hurdsig_preempted_set |= preemptor.signals; } - if (!hurd_siglocked) - __mutex_unlock (&_hurd_siglock); if (_hurd_itimer_port == MACH_PORT_NULL) { @@ -316,9 +313,6 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old, _hurd_itimer_thread_suspended = 0; } - __spin_unlock (&_hurd_itimer_lock); - _hurd_critical_section_unlock (crit); - if (old != NULL) { old->it_value = remaining; @@ -327,8 +321,6 @@ setitimer_locked (const struct itimerval *new, struct itimerval *old, return 0; out: - __spin_unlock (&_hurd_itimer_lock); - _hurd_critical_section_unlock (crit); return __hurd_fail (err); } @@ -339,7 +331,6 @@ int __setitimer (enum __itimer_which which, const struct itimerval *new, struct itimerval *old) { - void *crit; int ret; switch (which) @@ -356,9 +347,13 @@ __setitimer (enum __itimer_which which, const struct itimerval *new, } retry: - crit = _hurd_critical_section_lock (); + HURD_CRITICAL_BEGIN; + __mutex_lock (&_hurd_siglock); __spin_lock (&_hurd_itimer_lock); - ret = setitimer_locked (new, old, crit, 0); + ret = setitimer_locked (new, old); + __spin_unlock (&_hurd_itimer_lock); + __mutex_unlock (&_hurd_siglock); + HURD_CRITICAL_END; if (ret == -1 && errno == EINTR) /* Got a signal while inside an RPC of the critical section, retry again */ goto retry; @@ -373,12 +368,15 @@ fork_itimer (void) struct itimerval it; + __mutex_lock (&_hurd_siglock); __spin_lock (&_hurd_itimer_lock); _hurd_itimer_thread = MACH_PORT_NULL; it = _hurd_itimerval; it.it_value = it.it_interval; - setitimer_locked (&it, NULL, NULL, 0); + setitimer_locked (&it, NULL); + __spin_unlock (&_hurd_itimer_lock); + __mutex_unlock (&_hurd_siglock); (void) &fork_itimer; /* Avoid gcc optimizing out the function. */ }