Re: [libvirt] [PATCH v2 14/14] remote: add locking around the critical section in remoteSASLFinish

On 12/21/2017 09:29 AM, Marc Hartmayer wrote: > ...as there is an access to priv->sasl the priv->lock is needed. > > Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; > Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > daemon/remote.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > Both callers remoteDispatchAuthSaslStart and remoteDispatchAuthSaslStep already have priv->lock taken (unless I'm missing something).

John > diff --git a/daemon/remote.c b/daemon/remote.c > index b6fe6d8539ff..81d570b6e269 100644 > --- a/daemon/remote.c > +++ b/daemon/remote.c > @@ -3389,6 +3389,9 @@ remoteSASLFinish(virNetServerPtr server, > const char *identity; > struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); > int ssf; > + int rv = 0; > + > + virMutexLock(&priv->lock); > > /* TLS or UNIX domain sockets trivially OK */ > if (!virNetServerClientIsSecure(client)) { > @@ -3398,15 +3401,15 @@ remoteSASLFinish(virNetServerPtr server, > VIR_DEBUG("negotiated an SSF of %d", ssf); > if (ssf < 56) { /* 56 is good for Kerberos */ > VIR_ERROR(_("negotiated SSF %d was not strong enough"), ssf); > - return -2; > + goto rejected; > } > } > > if (!(identity = virNetSASLSessionGetIdentity(priv->sasl))) > - return -2; > + goto rejected; > > if (!virNetSASLContextCheckIdentity(saslCtxt, identity)) > - return -2; > + goto rejected; > > if (!(clnt_identity = virNetServerClientGetIdentity(client))) > goto error; > @@ -3425,10 +3428,17 @@ remoteSASLFinish(virNetServerPtr server, > virObjectUnref(priv->sasl); > priv->sasl = NULL; > > - return 0; > + cleanup: > + virMutexUnlock(&priv->lock); > + return rv; > > error: > - return -1; > + rv = -1; > + goto cleanup; > + > + rejected: > + rv = -2; > + goto cleanup; > } > > /* >