External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 13:45:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

remoteDispatchAuthPolkit: Fix lock ordering deadlock if client closes connection during auth

Browse Source 
Locks in following text:
A: virNetServer
B: virNetServerClient
C: daemonClientPrivate

'virNetServerSetClientAuthenticated' locks A then B

'remoteDispatchAuthPolkit' calls 'virNetServerSetClientAuthenticated'
while holding C.

If a client closes its connection 'virNetServerProcessClients' with the
lock A and B locked will call 'virNetServerClientCloseLocked' which will
try to dispose of the 'client' private data by:

  ref(b);
  unlock(b);
  remoteClientFreePrivateCallbacks();
  lock(b);
  unref(b);

Unfortunately remoteClientFreePrivateCallbacks() tries lock C.

Thus the locks are held in the following order:

 polkit auth: C -> A
 connection close: A -> C

causing a textbook-example deadlock. To resolve it we can simply drop
lock 'C' before calling 'virNetServerSetClientAuthenticated' as the lock
is not needed any more.

Resolves: https://issues.redhat.com/browse/RHEL-20337
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
This commit is contained in:
Peter Krempa 2024-01-17 15:55:35 +01:00
parent f95675fdbb
commit c697aff8a1
1 changed files with 42 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
src/remote/remote_daemon_dispatch.c
View File

@ -3979,50 +3979,52 @@ remoteDispatchAuthPolkit(virNetServer *server,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
int rv;
VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
action = virNetServerClientGetReadonly(client) ?
"org.libvirt.unix.monitor" :
"org.libvirt.unix.manage";
VIR_WITH_MUTEX_LOCK_GUARD(&priv->lock) {
action = virNetServerClientGetReadonly(client) ?
"org.libvirt.unix.monitor" :
"org.libvirt.unix.manage";
VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client));
if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
VIR_ERROR(_("client tried invalid PolicyKit init request"));
goto authfail;
VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client));
if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) {
VIR_ERROR(_("client tried invalid PolicyKit init request"));
goto authfail;
}
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
&callerPid, &timestamp) < 0) {
goto authfail;
}
if (timestamp == 0) {
VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start time",
(long long)callerPid);
goto authfail;
}
VIR_INFO("Checking PID %lld running as %d",
(long long) callerPid, callerUid);
rv = virPolkitCheckAuth(action,
callerPid,
timestamp,
callerUid,
NULL,
true);
if (rv == -1)
goto authfail;
else if (rv == -2)
goto authdeny;
PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
"client=%p auth=%d identity=%s",
client, REMOTE_AUTH_POLKIT, ident);
VIR_INFO("Policy allowed action %s from pid %lld, uid %d",
action, (long long) callerPid, callerUid);
ret->complete = 1;
}
if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid,
&callerPid, &timestamp) < 0) {
goto authfail;
}
if (timestamp == 0) {
VIR_WARN("Failing polkit auth due to missing client (pid=%lld) start time",
(long long)callerPid);
goto authfail;
}
VIR_INFO("Checking PID %lld running as %d",
(long long) callerPid, callerUid);
rv = virPolkitCheckAuth(action,
callerPid,
timestamp,
callerUid,
NULL,
true);
if (rv == -1)
goto authfail;
else if (rv == -2)
goto authdeny;
PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
"client=%p auth=%d identity=%s",
client, REMOTE_AUTH_POLKIT, ident);
VIR_INFO("Policy allowed action %s from pid %lld, uid %d",
action, (long long) callerPid, callerUid);
ret->complete = 1;
/* this must be called with the private data mutex unlocked */
virNetServerSetClientAuthenticated(server, client);
return 0;