Re: [PATCH] nptl: Use a PI-aware lock for internal pthread_cond-related locking

[Florian Weimer <fweimer@redhat.com>]

* Sebastian Andrzej Siewior:

>> Are there seccomp filters for PI futexes?  I wouldn't be surprised if
>> people added them after some of the high-profile futex vulnerabilities.
>> I think we should not support such seccomp filters, but we need to know
>> what we are up against.
>
> I don't know. But don't you allow a syscall such a sys_futex and don't
> filter additional arguments? Unless one would filter the op argument, it
> shouldn't be an issue.

There's this historic example:

  Remove priority inheritance support for Futex in all Chrome policies,
  including NaCl NonSFI
  <https://issues.chromium.org/issues/40381864>

Some applications are exactly doing that, though, particularly for
FUTEX_CMP_REQUEUE_PI.

Given how widely the Chromium sandbox code is used, I wonder if this is
still an issue.

>> > There shouldn't be any error. There might be the case where the lock
>> > owner is gone (ESRCH I believe) or the theoretical ENOMEM. ESRCH isn't
>> > handled now but it can't be recognized either. It would require to kill
>> > the thread owning the lock.
>> > So either abort the operation if the futex-op returns an error because
>> > "this shouldn't happen" or I don't know.
>> 
>> ENOMEM needs to be reported to the caller because the application may
>> want to react to it.
>
> Well. Right now it only checks ESRCH and EDEADLK. Everything else is
> considered success.
> So do want to update this + man-page?

The man page already mentions ENOMEM, it's just glibc that is buggy.

> But what should be done this pthread_cond_.*() functions? I guess we
> can't forward that possible -ENOMEM to the caller?

Is this about the wait operation?  POSIX allows spurious wakeups, so we
could technically return without an error.

> Also if we are in good mood, there pthread_mutex_lock() has this comment
> |                 /* ESRCH can happen only for non-robust PI mutexes where
> |                    the owner of the lock died.  */
>
> This is simply not true as far as the kernel goes. If the futex uaddr
> contains a pid of a non-existing task then LOCK_PI will return ESRCH. A
> simple testcase would be
>
> | #include <stdio.h>
> | #include <pthread.h>
> |
> | static pthread_mutex_t l;
> |
> | static void *thread_code(void *arg)
> | {
> |         pthread_mutex_lock(&l);
> |         return NULL;
> | }
> |
> | int main(void)
> | {
> |         pthread_mutexattr_t attr;
> |         pthread_t thread;
> |         int ret;
> |
> |         ret = pthread_mutexattr_init(&attr);
> |         ret |= pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT);
> |         ret |= pthread_mutex_init(&l, &attr);
> |         ret |= pthread_create(&thread, NULL, thread_code, NULL);
> |         if (ret) {
> |                 printf("->[%d] %d\n", __LINE__, ret);
> |                 return 1;
> |         }
> |         pthread_join(thread, NULL);
> |         ret = pthread_mutex_lock(&l);
> |         printf("-> %d %m\n", ret);
> |
> |         return 0;
> | }
>
> and strace says:
> | futex(0x55afb0ceb080, FUTEX_LOCK_PI_PRIVATE, NULL) = -1 ESRCH (No such process)
> | futex(0x7ffe73216f24, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 0, NULL, FUTEX_BITSET_MATCH_ANY
>
> the second futex() is the __futex_abstimed_wait64() that follows.

Does POSIX define what happens if a non-robust mutex is accessed again
if a thread terminates without unlocking it first?  If yes, then this is
indeed a problem.

Thanks,
Florian