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

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > ---
> >   src/rpc/virnetsaslcontext.c | 50 ++++++++++++++++++++++++-------------
> >   1 file changed, 33 insertions(+), 17 deletions(-)
> 
> Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> 
> 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;
}

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list