nptl: Propagate kernel EDEADLK and clear robust list on PI mutex deadlock

Message ID 20260901140337.3581214-1-adhemerval.zanella@linaro.org (mailing list archive)
State New
Headers
Series nptl: Propagate kernel EDEADLK and clear robust list on PI mutex deadlock |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Adhemerval Zanella Netto Sept. 1, 2026, 2:03 p.m. UTC
  The kernel FUTEX_LOCK_PI deadlock detection is handled
inconsistently.  The pthread_mutex_lock passes EDEADLK through for
error-checking mutexes without clearing the pending robust list
operation (set for robust mutexes, allowing the same corruption at
thread exit as bug 34542), while pthread_mutex_timedlock still
asserted the error could not happen and aborted.

This was a missing spot add by 0e30e9ce27 not handled by 69841bbaa5.

Also tuned down tst-deadlk.c timers to avoid it taking too much time.
With the inclusion on the new tests total time was about 42s on
recent x86_64 box, it is not around 9s.

Checked on x86_64-linux-gnu.
---
 nptl/pthread_mutex_lock.c      |   2 +-
 nptl/pthread_mutex_timedlock.c |  14 +---
 nptl/tst-deadlk.c              |  92 ++++++++++++++++-----
 nptl/tst-robust12.c            | 144 +++++++++++++++++++++++++++++++++
 sysdeps/nptl/pthreadP.h        |   7 ++
 5 files changed, 226 insertions(+), 33 deletions(-)
  

Patch

diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
index 3e82eccf3b..0603fc165c 100644
--- a/nptl/pthread_mutex_lock.c
+++ b/nptl/pthread_mutex_lock.c
@@ -425,7 +425,7 @@  __pthread_mutex_lock_full (pthread_mutex_t *mutex)
 		       check above.  Pass this error through for error-checking
 		       mutexes; otherwise, intentionally deadlock for all other
 		       mutex types.  */
-		    return e;
+		    return __pthread_mutex_robust_error (e);
 		  }
 
 		/* ESRCH can happen only for non-robust PI mutexes where
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 2a44108736..0bcf327fd3 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -28,14 +28,6 @@ 
 
 #include <stap-probe.h>
 
-/* Return the error code ERR after clearing the robust list head.  */
-static int
-__pthread_mutex_robust_error (int err)
-{
-  THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
-  return err;
-}
-
 int
 __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 				  clockid_t clockid,
@@ -352,9 +344,9 @@  __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 	      return __pthread_mutex_robust_error (ETIMEDOUT);
 	    else if (e == ESRCH || e == EDEADLK)
 	      {
-		assert (e != EDEADLK
-			|| (kind != PTHREAD_MUTEX_ERRORCHECK_NP
-			   && kind != PTHREAD_MUTEX_RECURSIVE_NP));
+		if (e == EDEADLK && kind == PTHREAD_MUTEX_ERRORCHECK_NP)
+		  return __pthread_mutex_robust_error (e);
+
 		/* ESRCH can happen only for non-robust PI mutexes where
 		   the owner of the lock died.  */
 		assert (e != ESRCH || !robust);
diff --git a/nptl/tst-deadlk.c b/nptl/tst-deadlk.c
index bf787826ae..913dd62810 100644
--- a/nptl/tst-deadlk.c
+++ b/nptl/tst-deadlk.c
@@ -25,7 +25,9 @@ 
 #include <support/check.h>
 #include <support/support.h>
 #include <support/test-driver.h>
+#include <support/timespec.h>
 #include <support/xthread.h>
+#include <support/xtime.h>
 
 #define ASSUME_DEADLOCK_AFTER_SECONDS 3
 
@@ -34,12 +36,16 @@  struct which_mutex
   int type;
   bool prio_inherit;
   bool robust;
+  bool timed;
 };
 
 struct task_context
 {
   pthread_mutex_t *first, *second;
   pthread_barrier_t *barrier;
+  bool timed;
+  /* Relative timeout for the timed variant.  */
+  struct timespec timeout;
 };
 
 static bool
@@ -55,7 +61,17 @@  thread_function (void *const arg)
   intptr_t ret = 0;
   xpthread_mutex_lock (ctx->first);
   xpthread_barrier_wait (ctx->barrier);
-  ret = pthread_mutex_lock (ctx->second);
+  if (ctx->timed)
+    {
+      /* The timeout is not expected to be reached: the kernel either
+         reports the deadlock right away or grants the lock once the
+         other thread got EDEADLK and released it.  */
+      struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+                                        ctx->timeout);
+      ret = pthread_mutex_timedlock (ctx->second, &t);
+    }
+  else
+    ret = pthread_mutex_lock (ctx->second);
   xpthread_mutex_unlock (ctx->first);
   if (ret == 0)
     xpthread_mutex_unlock (ctx->second);
@@ -84,26 +100,41 @@  do_test_single (void *const ctx)
   pthread_barrier_t barrier;
   const struct which_mutex *const that = ctx;
   const bool graceful = should_detect_deadlock (that);
+  const struct timespec timeout = graceful
+      ? make_timespec (5, 0) : make_timespec (0, 200000000);
   struct task_context ctx1
-      = { .first = &m1, .second = &m2, .barrier = &barrier };
+      = { .first = &m1, .second = &m2, .barrier = &barrier,
+	  .timed = that->timed, .timeout = timeout };
   struct task_context ctx2
-      = { .first = &m2, .second = &m1, .barrier = &barrier };
+      = { .first = &m2, .second = &m1, .barrier = &barrier,
+	  .timed = that->timed, .timeout = timeout };
   xpthread_barrier_init (&barrier, NULL, 2);
   prepare_mutex (&m1, that);
   prepare_mutex (&m2, that);
   const pthread_t t1 = xpthread_create (NULL, thread_function, &ctx1);
   const pthread_t t2 = xpthread_create (NULL, thread_function, &ctx2);
-  if (!graceful)
+  if (!graceful && !that->timed)
     delayed_exit (ASSUME_DEADLOCK_AFTER_SECONDS);
   const int ret1 = (intptr_t) xpthread_join (t1);
   const int ret2 = (intptr_t) xpthread_join (t2);
   xpthread_mutex_destroy (&m1);
   xpthread_mutex_destroy (&m2);
   xpthread_barrier_destroy (&barrier);
-  TEST_VERIFY (graceful);
-  TEST_VERIFY (ret1 == 0 || ret1 == EDEADLK);
-  TEST_VERIFY (ret2 == 0 || ret2 == EDEADLK);
-  TEST_VERIFY (ret1 == EDEADLK || ret2 == EDEADLK);
+  if (that->timed && !graceful)
+    {
+      /* The deadlock is not detected: the lock attempts block until the
+	 timeout expires.  */
+      TEST_VERIFY (ret1 == 0 || ret1 == ETIMEDOUT);
+      TEST_VERIFY (ret2 == 0 || ret2 == ETIMEDOUT);
+      TEST_VERIFY (ret1 == ETIMEDOUT || ret2 == ETIMEDOUT);
+    }
+  else
+    {
+      TEST_VERIFY (graceful);
+      TEST_VERIFY (ret1 == 0 || ret1 == EDEADLK);
+      TEST_VERIFY (ret2 == 0 || ret2 == EDEADLK);
+      TEST_VERIFY (ret1 == EDEADLK || ret2 == EDEADLK);
+    }
 }
 
 static int
@@ -121,19 +152,38 @@  do_test (void)
         {
           for (int rb = 0; rb < 2; ++rb)
             {
-              struct which_mutex that = {
-                .type = mutex_types[i],
-                .prio_inherit = pi,
-                .robust = rb,
-              };
-              const char *const description
-                  = xasprintf ("type = %d, prio_inherit = %d, robust = %d",
-                               that.type, that.robust, that.prio_inherit);
-              struct support_capture_subprocess capture
-                  = support_capture_subprocess (do_test_single, &that);
-              support_capture_subprocess_check (&capture, description, 0,
-                                                sc_allow_none);
-              support_capture_subprocess_free (&capture);
+              for (int timed = 0; timed < 2; ++timed)
+                {
+                  struct which_mutex that = {
+                    .type = mutex_types[i],
+                    .prio_inherit = pi,
+                    .robust = rb,
+                    .timed = timed,
+                  };
+                  /* The timed variant covers all combinations: where the
+                     kernel does not detect the deadlock, expiring the
+                     timeout is the expected way out.  The plain
+                     pthread_mutex_lock variant genuinely deadlocks in
+                     those combinations and costs
+                     ASSUME_DEADLOCK_AFTER_SECONDS each, so run it only
+                     where the deadlock is detected, plus one
+                     representative deadlocking combination per PI
+                     setting.  */
+                  if (!timed && !should_detect_deadlock (&that)
+                      && !(that.type == PTHREAD_MUTEX_TIMED_NP
+                           && !that.robust))
+                    continue;
+                  const char *const description
+                      = xasprintf ("type = %d, prio_inherit = %d, "
+                                   "robust = %d, timed = %d",
+                                   that.type, that.prio_inherit, that.robust,
+                                   that.timed);
+                  struct support_capture_subprocess capture
+                      = support_capture_subprocess (do_test_single, &that);
+                  support_capture_subprocess_check (&capture, description, 0,
+                                                    sc_allow_none);
+                  support_capture_subprocess_free (&capture);
+                }
             }
         }
     }
diff --git a/nptl/tst-robust12.c b/nptl/tst-robust12.c
index 0da0a358af..0f981ac4ec 100644
--- a/nptl/tst-robust12.c
+++ b/nptl/tst-robust12.c
@@ -148,6 +148,142 @@  test_one (bool use_pi)
   xpthread_barrier_destroy (&barrier);
 }
 
+
+/* Check if the pthread clear pending robut list for the kernel detected
+   deadlock cases.  */
+
+static pthread_mutex_t mutex2;
+
+/* Fro the kABI futex interface.  */
+#define FUTEX_WAITERS 0x80000000
+
+struct deadlock_args
+{
+  bool use_timedlock;
+  int result;
+};
+
+static void *
+deadlock_threadfunc (void *closure)
+{
+  struct deadlock_args *args = closure;
+
+  xpthread_mutex_lock (&mutex);
+  futex_value = mutex.__data.__lock;
+  xpthread_mutex_unlock (&mutex);
+  xpthread_barrier_wait (&barrier);
+
+  xpthread_barrier_wait (&barrier);
+  xpthread_mutex_lock (&mutex2);
+  xpthread_barrier_wait (&barrier);
+
+  while ((__atomic_load_n (&mutex2.__data.__lock, __ATOMIC_RELAXED)
+	  & FUTEX_WAITERS) == 0)
+    nanosleep (&(struct timespec) { 0, 100000 }, NULL);
+
+  /* For the case where the kernel has not yet see the main thread as blocked
+     (schedule pressure, system load, main thread has timed out, etc), the
+     deadload is not reported and the lock attempt succeeds.  So retry for
+     such cases.  */
+  int ret;
+  if (args->use_timedlock)
+    {
+      /* The timeout is not expected to be triggered, the kernel should
+	 report the deadlock.  */
+      struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+					make_timespec (4, 0));
+      ret = pthread_mutex_timedlock (&mutex, &t);
+    }
+  else
+    ret = pthread_mutex_lock (&mutex);
+  args->result = ret;
+  if (ret == 0)
+    xpthread_mutex_unlock (&mutex);
+
+  xpthread_barrier_wait (&barrier);
+  xpthread_barrier_wait (&barrier);
+
+  return NULL;
+}
+
+enum { no_deadloadk_attempts = 16 };
+
+static void
+test_deadlock (bool use_timedlock)
+{
+  struct deadlock_args args = { .use_timedlock = use_timedlock };
+
+  /* Retry in the case the deadlock is not reported to the test thread.  */
+  for (int attempt = 0; attempt < no_deadloadk_attempts; ++attempt)
+    {
+      xpthread_barrier_init (&barrier, NULL, 2);
+
+      {
+	pthread_mutexattr_t a;
+	xpthread_mutexattr_init (&a);
+	TEST_COMPARE (pthread_mutexattr_setrobust (&a, PTHREAD_MUTEX_ROBUST),
+		      0);
+	/* NB: error checkin, so that kernel detects the deadlock.  */
+	xpthread_mutexattr_settype (&a, PTHREAD_MUTEX_ERRORCHECK);
+	xpthread_mutexattr_setprotocol (&a, PTHREAD_PRIO_INHERIT);
+	xpthread_mutex_init (&mutex, &a);
+	xpthread_mutex_init (&mutex2, &a);
+	xpthread_mutexattr_destroy (&a);
+      }
+
+      args.result = -1;
+      pthread_t thr = xpthread_create (NULL, deadlock_threadfunc, &args);
+
+      xpthread_barrier_wait (&barrier);
+
+      xpthread_mutex_lock (&mutex);
+      /* Allow the test thread to lock MUTEX2.  */
+      xpthread_barrier_wait (&barrier);
+      /* Wait for the test thread to lock MUTEX2.  */
+      xpthread_barrier_wait (&barrier);
+
+      struct timespec t = timespec_add (xclock_now (CLOCK_REALTIME),
+					make_timespec (2, 0));
+      int ret = pthread_mutex_timedlock (&mutex2, &t);
+      TEST_VERIFY (ret == ETIMEDOUT || ret == EDEADLK);
+
+      /* In the expected case the target should have received EDEADLK,
+         otherwise the kernel reported the deadlock to this thread instead,
+	 or the timeout expired efore thre test thread attempted the lock.
+	 For the later retry the round.  */
+      xpthread_mutex_unlock (&mutex);
+
+      /* Wait for the test thread's lock attempt to finish.  */
+      xpthread_barrier_wait (&barrier);
+
+      if (args.result != 0)
+	TEST_COMPARE (args.result, EDEADLK);
+
+      /* Destroy MUTEX and overwrite it with a different bit pattern than a
+	 valid futex value that will be acted upon by the kernel if the
+	 robust list of the exiting test thread still references the mutex.
+	 MUTEX2 is still owned by the test thread and is released by the
+	 kernel when the thread exits.  */
+      xpthread_mutex_destroy (&mutex);
+      char pattern[sizeof (mutex)];
+      memset (pattern, 0xcc, sizeof (pattern));
+      memcpy (&pattern[offsetof (pthread_mutex_t, __data.__lock)],
+	      &futex_value, sizeof (futex_value));
+      memcpy (&mutex, pattern, sizeof (mutex));
+
+      xpthread_barrier_wait (&barrier);
+      xpthread_join (thr);
+
+      TEST_COMPARE_BLOB (&mutex, sizeof (mutex), pattern, sizeof (pattern));
+
+      xpthread_barrier_destroy (&barrier);
+
+      if (args.result == EDEADLK)
+	return;
+    }
+  FAIL_EXIT1 ("deadlock was never reported to the test thread");
+}
+
 static int
 do_test (void)
 {
@@ -160,6 +296,14 @@  do_test (void)
         test_one (use_pi);
       }
 
+  for (int use_timedlock = 0; use_timedlock < 2; ++use_timedlock)
+    {
+      printf ("info: deadlock subtest, %s\n",
+	      use_timedlock ? "pthread_mutex_timedlock"
+			    : "pthread_mutex_lock");
+      test_deadlock (use_timedlock);
+    }
+
   return 0;
 }
 
diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
index 982705aee5..bfe184a9ca 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -214,6 +214,13 @@  libc_hidden_proto (__pthread_current_priority)
 /* If OLDPRIO is not -1, undo its TPP priority change.  Return ERR.  */
 int __pthread_mutex_priority_error (int err, int oldprio) attribute_hidden;
 
+static __always_inline int
+__pthread_mutex_robust_error (int err)
+{
+  THREAD_SETMEM (THREAD_SELF, robust_head.list_op_pending, NULL);
+  return err;
+}
+
 
 /* This will not catch all invalid descriptors but is better than
    nothing.  And if the test triggers the thread descriptor is