Re: [libvirt] [PATCH] rpc: ensure thread safe initialization of SASL library

On Mon, Jul 08, 2019 at 01:12:06PM +0200, Michal Privoznik wrote: > On 7/8/19 12:39 PM, Daniel P. Berrangé wrote: > > Neither the sasl_client_init or sasl_server_init methods are even > > remotely threadsafe. They do a bunch of one-time initialization and > > merely use a simple integer counter to avoid repeated work, not even > > using atomic increment/reads on the counter. This can easily race in a > > threaded program. Protect the calls using a virOnce initializer function > > which is guaranteed threadsafe at least from libvirt's POV. > > > > If the application using libvirt also uses another library that makes > > use of SASL then the race still exists. It is impossible to fix that > > fully except in SASL code itself. > > > > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > > --- > > src/rpc/virnetsaslcontext.c | 50 ++++++++++++++++++++++++------------- > > 1 file changed, 33 insertions(+), 17 deletions(-) > > Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > > Thanks Sahid for the report! FYI i wrote a simple demo program that can reliably reproduce the problem in isolation

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

#include <libvirt/libvirt.h> #include <stdio.h> #include <pthread.h> #include <unistd.h> pthread_cond_t condReady; pthread_cond_t condRun; pthread_mutex_t lock; int running; void *runme(void *arg) { virConnectPtr conn; pthread_mutex_lock(&lock); running++; fprintf(stderr, "Notifying we are ready\n"); pthread_cond_signal(&condReady); pthread_cond_wait(&condRun, &lock); pthread_mutex_unlock(&lock); fprintf(stderr, "Opening libvirt\n"); conn = virConnectOpenAuth("qemu:///system", virConnectAuthPtrDefault, 0); fprintf(stderr, "Open %s\n", conn ? virConnectGetURI(conn) : "failed"); if (conn) virConnectClose(conn); } int main(int argc, char **argv) { int i; pthread_t th[20]; pthread_mutex_init(&lock, NULL); pthread_cond_init(&condRun, NULL); pthread_cond_init(&condReady, NULL); for (i = 0; i < 20; i++) { pthread_create(&th[i], NULL, runme, NULL); } fprintf(stderr, "Waiting for threads to initialize\n"); pthread_mutex_lock(&lock); while (running != 20) pthread_cond_wait(&condReady, &lock); fprintf(stderr, "Telling threads to proceed\n"); pthread_cond_broadcast(&condRun); pthread_mutex_unlock(&lock); for (i = 0; i < 20; i++) { pthread_join(th[i], NULL); } return 0; }