Skip to content

Commit

Permalink
Fallback to a read lock on fork preparation.
Browse files Browse the repository at this point in the history
The reason to acquire a write lock is that there is a corner case where
the forking application is going to actually create additional threads
in an atfork() handler that runs before ours instead of doing something
more sensible like an immediate exec().
If such an application were to create threads then there is a chance
one of those threads could cause a call into pkcs11-provider that would
end up acquiring a read lock, which we will destroy as part of the post
fork atfork() handler.
Note that pthread creation or even pthread lock initialization functions
are not considered async-sginal-safe fucntions, so this is all a shot in
the dark anyway ...

Given this premise, falling back to a read lock, which will prevent
modification of the slots is sufficient in most cases.

Although there is a slight chance again that resetting the locks can
cause a thread to obtain a write lock while another thread holds a now
defunct read lock, such that there is modification of the slots while
a separate thread is reading this information, this is so rare it is
not worth dealing with (and there is nothing we can do anyway).
At least two threads won't be writing at the same time, that could be
really problematic.

Use of atfork() is also so rare that the chance of this happening in any
reasonable application can be considered infinitesimally small.

Signed-off-by: Simo Sorce <simo@redhat.com>
  • Loading branch information
simo5 committed Mar 8, 2024
1 parent 58040b4 commit 71f0fee
Showing 1 changed file with 20 additions and 4 deletions.
24 changes: 20 additions & 4 deletions src/slot.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,27 @@ void p11prov_slot_fork_prepare(P11PROV_SLOTS_CTX *sctx)
{
int err;

err = pthread_rwlock_wrlock(&sctx->rwlock);
/* attempt to get a write lock if possible, but fall back to a mere
* read lock if not possible (for example because it would cause
* a deadlock.
* Holding a write lock here is slightly prefereable in case the
* application decides to create threads immediately after the fork
* within an atfork handler that runs before ours.
* Holding a write lock will prevent other threads from grabbing a
* read lock before we can reset the locks. However we assume this
* scenario to be mostly hypotethical and exeedeingly rare given most
* forks result in a exec(), and atfork() is also a rarely used
* function, so falling back to a read lock to avoid deadlocks is ok
* in the vast majority of use cases.
*/
err = pthread_rwlock_trywrlock(&sctx->rwlock);
if (err != 0) {
err = errno;
P11PROV_debug("Failed to get slots lock (errno:%d)", err);
return;
err = pthread_rwlock_rdlock(&sctx->rwlock);
if (err != 0) {
err = errno;
P11PROV_debug("Failed to get slots lock (errno:%d)", err);
return;
}
}
}

Expand Down

0 comments on commit 71f0fee

Please sign in to comment.