From 18b058c78d0e26e4a10b4678ce2a11f52084c017 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 7 Mar 2024 16:42:18 -0500 Subject: [PATCH] Fallback to a read lock on fork preparation. 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-signal-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 --- src/slot.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/slot.c b/src/slot.c index 6a936028..b6f80bd4 100644 --- a/src/slot.c +++ b/src/slot.c @@ -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 preferable 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 hypothetical and exceedingly 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; + } } }