[Bug libgomp/126791] New: linux/ target use-after-free possibility with `futex_wake` in `gomp_barrier_wait_last`

"matmal01 at gcc dot gnu.org via Gcc-bugs" <[email protected]>
Newsgroups gmane.comp.gcc.bugs
Message-ID <[email protected]/bugzilla/>
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126791

            Bug ID: 126791
           Summary: linux/ target use-after-free possibility with
                    `futex_wake` in `gomp_barrier_wait_last`
           Product: gcc
           Version: 16.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgomp
          Assignee: unassigned at gcc dot gnu.org
          Reporter: matmal01 at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

`gomp_team_end` is used to end teams.

When it's ending a nested team it uses synchronization based on
`gomp_barrier_wait_last` and `gomp_barrier_wait` in order to ensure that all
secondary threads have entered the barrier before the primary thread running
this function frees the team (including the barrier).

A secondary thread calling `gomp_barrier_wait_last` may be the last thread to
enter the barrier.
In this case said secondary would be the thread to signal barrier completion by
incrementing `bar->generation`.
After it signals barrier completion it performs `futex_wake` on that same
location.

The gap between the increment of the generation and the `futex_wake` could
technically allow the primary thread to:
1) Notice that the barrier has completed.
2) Progress through `gomp_team_end`, and call `free` on the team (containing
the barrier).

In this case the secondary thread would call `futex_wake` on some memory that
has been freed.

------------
I suspect the cleanest way to fix this would be to introduce something like
`gomp_barrier_wait_until_last` to pair with `gomp_barrier_wait_last`.  This
interface could avoids the race condition by having the `wait_until_last`
version not call `futex_wait` and correspondingly the code path where the
secondary could increment `bar->generation` from `wait_last` not call
`futex_wake`.

My rationale being that permanent spinning only on the primary thread and for
what should be a short time (the secondary threads not performing any work in
between the last barrier and the `_last` barrier) would be preferred to a racey
possibility of use-after-free.

A similar problem can be seen in `gomp_free_thread` and `gomp_pause_host` (that
use the `gomp_barrier_wait_last` sync).

------------
A slightly different version of the same problem can be seen with a worker
excluded by a team-shrink where the secondary increments the
`pool->threads_dock` generation and there is no synchronization with this
secondary and any other threads before the `futex_wake` that the secondary
could perform.
If the pool gets freed before the `futex_wake` (e.g. with `gomp_pause_host`),
then the secondary can call `futex_wake` on a freed piece of data.

This is a little more tricky to handle, since it's not using the existing
`gomp_barrier_wait_last` interface and the barrier wait is also in the
hot-path.
FWIW I suspect that with the `linux/futex_waitv` target that I'm working on the
fact that only threads starting or exiting use the `threads_dock` barrier would
make the fix more straight-forward for that target (I believe could use
something like the `wait_last` versions when we know the given thread is
exiting).
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.