[OpenAFS-devel] deadlock in host.c (fileserver)
Jeffrey Altman
jaltman@secure-endpoints.com
Tue, 20 Jun 2006 08:32:31 -0400
This is a cryptographically signed message in MIME format.
--------------ms000505020403050404010907
Content-Type: multipart/mixed;
boundary="------------030309060808070709040400"
This is a multi-part message in MIME format.
--------------030309060808070709040400
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Rainer:
Thank you for the patch. The second block of your patch is indeed a
deadlock that must be fixed. The first block of your patch affects
code that has been changed on the 1.4.x branch. Please examine the
attached patch.
Jeffrey Altman
Rainer Toebbicke wrote:
> There is a possible deadlock in host.c in the fileserver code:
>
> around line 1882, where the lock for client is dropped in order to lock
> oldClient, it is not impossible (e.g. through prfail == 2, or just if
> nothing exciting happened through pr_GetCPS) that client == oldClient.
>
> In itself "switching" at that point is already fishy.
>
> Worse however, the H_LOCK is held. When the lock for client is dropped,
> another thread can grab it and immediately after that queue for the
> H_LOCK. Since we hold H_LOCK we'll deadlock if we now wait for the
> oldClient's lock since the other thread's got it.
>
> The attached patch ...
>
> 1. correctly handles client == oldClient
>
> 2. drops the H_LOCK when switching to oldClient, with the reference
> count updated before,
>
> 3. does all this in a re-check loop since both locks are dropped at one
> point.
>
> (Bcc'ed to openafs-bugs)
--------------030309060808070709040400
Content-Type: text/plain;
name="viced-rt34073-diff-2.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="viced-rt34073-diff-2.txt"
? AFS_component_version_number.h
? rs_state.ini
? viced-rt34073-diff-2.txt
Index: host.c
===================================================================
RCS file: /cvs/openafs/src/viced/host.c,v
retrieving revision 1.57.2.36
diff -u -r1.57.2.36 host.c
--- host.c 7 Jun 2006 04:55:25 -0000 1.57.2.36
+++ host.c 20 Jun 2006 12:30:48 -0000
@@ -1759,7 +1759,7 @@
* the RPC from the other client structure's rock.
*/
oldClient = (struct client *)rx_GetSpecific(tcon, rxcon_client_key);
- if (oldClient && oldClient->sid == rxr_CidOf(tcon)
+ if (oldClient && oldClient != client && oldClient->sid == rxr_CidOf(tcon)
&& oldClient->VenusEpoch == rxr_GetEpoch(tcon)) {
char hoststr[16];
if (!oldClient->deleted) {
@@ -1782,8 +1782,10 @@
FreeCE(client);
created = 0;
}
- ObtainWriteLock(&oldClient->lock);
oldClient->refCount++;
+ H_UNLOCK;
+ ObtainWriteLock(&oldClient->lock);
+ H_LOCK;
client = oldClient;
} else {
ViceLog(0, ("FindClient: deleted client %x(%x) already had conn %x (host %s:%d), stolen by client %x(%x)\n",
--------------030309060808070709040400--
--------------ms000505020403050404010907
Content-Type: application/x-pkcs7-signature; name="smime.p7s"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="smime.p7s"
Content-Description: S/MIME Cryptographic Signature
MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIIJeTCC
AxcwggKAoAMCAQICEBW00lKwoWJXt8wbmTl1M0kwDQYJKoZIhvcNAQEEBQAwYjELMAkGA1UE
BhMCWkExJTAjBgNVBAoTHFRoYXd0ZSBDb25zdWx0aW5nIChQdHkpIEx0ZC4xLDAqBgNVBAMT
I1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFpbCBJc3N1aW5nIENBMB4XDTA2MDUyNzIyMDMzMloX
DTA3MDUyNzIyMDMzMlowczEPMA0GA1UEBBMGQWx0bWFuMRUwEwYDVQQqEwxKZWZmcmV5IEVy
aWMxHDAaBgNVBAMTE0plZmZyZXkgRXJpYyBBbHRtYW4xKzApBgkqhkiG9w0BCQEWHGphbHRt
YW5Ac2VjdXJlLWVuZHBvaW50cy5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB
AQC19SD7DncCP/+wfQlLzAAcxf1nJ/7UQgh4o/nxzvuY55XwHdLQjqWuFUnM5vecfyZKwq0o
fGCucDfcQbSIrkhHD9z4TZ8vDaYWVY9Foz8Rp8G0PNdbRFoFtfJbaeVBX5hG3aQXIc/T1b9U
8uN3kLyqXAFIGWKO8DJVGTKKtOiPVOp1U+9CwujyYmUSKF+suutKABhhK1ZGHsTnFczLZ2g0
ma0H7PiFJ2kLfOf///07E1fbr4IRb+cd87kpWLcjtEZ0rbBr9HlOy9dkeEii/qFoo1ahfKCD
A9bNErMiOXA3dudaNNzXlN/70slq5fboBXbepamJGrsnXYcCsS9+LtCTAgMBAAGjOTA3MCcG
A1UdEQQgMB6BHGphbHRtYW5Ac2VjdXJlLWVuZHBvaW50cy5jb20wDAYDVR0TAQH/BAIwADAN
BgkqhkiG9w0BAQQFAAOBgQDBzWhkrW+ol3iyT1rV8ZBQB0+z/6dFH3djQfNf7jDXNoXx4Vbo
pA7BAR4ihAPibv7j7ZaxmyMxWiDACRGS934uvUS0K6L6q14hTWMostJgFsAEDArrmbrES03v
L3EVETiGFqTB2sLp5MLc6+z+72pLXRuDPL3lO2GOQuBbILswRzCCAxcwggKAoAMCAQICEBW0
0lKwoWJXt8wbmTl1M0kwDQYJKoZIhvcNAQEEBQAwYjELMAkGA1UEBhMCWkExJTAjBgNVBAoT
HFRoYXd0ZSBDb25zdWx0aW5nIChQdHkpIEx0ZC4xLDAqBgNVBAMTI1RoYXd0ZSBQZXJzb25h
bCBGcmVlbWFpbCBJc3N1aW5nIENBMB4XDTA2MDUyNzIyMDMzMloXDTA3MDUyNzIyMDMzMlow
czEPMA0GA1UEBBMGQWx0bWFuMRUwEwYDVQQqEwxKZWZmcmV5IEVyaWMxHDAaBgNVBAMTE0pl
ZmZyZXkgRXJpYyBBbHRtYW4xKzApBgkqhkiG9w0BCQEWHGphbHRtYW5Ac2VjdXJlLWVuZHBv
aW50cy5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC19SD7DncCP/+wfQlL
zAAcxf1nJ/7UQgh4o/nxzvuY55XwHdLQjqWuFUnM5vecfyZKwq0ofGCucDfcQbSIrkhHD9z4
TZ8vDaYWVY9Foz8Rp8G0PNdbRFoFtfJbaeVBX5hG3aQXIc/T1b9U8uN3kLyqXAFIGWKO8DJV
GTKKtOiPVOp1U+9CwujyYmUSKF+suutKABhhK1ZGHsTnFczLZ2g0ma0H7PiFJ2kLfOf///07
E1fbr4IRb+cd87kpWLcjtEZ0rbBr9HlOy9dkeEii/qFoo1ahfKCDA9bNErMiOXA3dudaNNzX
lN/70slq5fboBXbepamJGrsnXYcCsS9+LtCTAgMBAAGjOTA3MCcGA1UdEQQgMB6BHGphbHRt
YW5Ac2VjdXJlLWVuZHBvaW50cy5jb20wDAYDVR0TAQH/BAIwADANBgkqhkiG9w0BAQQFAAOB
gQDBzWhkrW+ol3iyT1rV8ZBQB0+z/6dFH3djQfNf7jDXNoXx4VbopA7BAR4ihAPibv7j7Zax
myMxWiDACRGS934uvUS0K6L6q14hTWMostJgFsAEDArrmbrES03vL3EVETiGFqTB2sLp5MLc
6+z+72pLXRuDPL3lO2GOQuBbILswRzCCAz8wggKooAMCAQICAQ0wDQYJKoZIhvcNAQEFBQAw
gdExCzAJBgNVBAYTAlpBMRUwEwYDVQQIEwxXZXN0ZXJuIENhcGUxEjAQBgNVBAcTCUNhcGUg
VG93bjEaMBgGA1UEChMRVGhhd3RlIENvbnN1bHRpbmcxKDAmBgNVBAsTH0NlcnRpZmljYXRp
b24gU2VydmljZXMgRGl2aXNpb24xJDAiBgNVBAMTG1RoYXd0ZSBQZXJzb25hbCBGcmVlbWFp
bCBDQTErMCkGCSqGSIb3DQEJARYccGVyc29uYWwtZnJlZW1haWxAdGhhd3RlLmNvbTAeFw0w
MzA3MTcwMDAwMDBaFw0xMzA3MTYyMzU5NTlaMGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxU
aGF3dGUgQ29uc3VsdGluZyAoUHR5KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwg
RnJlZW1haWwgSXNzdWluZyBDQTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAxKY8VXNV
+065yplaHmjAdQRwnd/p/6Me7L3N9VvyGna9fww6YfK/Uc4B1OVQCjDXAmNaLIkVcI7dyfAr
hVqqP3FWy688Cwfn8R+RNiQqE88r1fOCdz0Dviv+uxg+B79AgAJk16emu59l0cUqVIUPSAR/
p7bRPGEEQB5kGXJgt/sCAwEAAaOBlDCBkTASBgNVHRMBAf8ECDAGAQH/AgEAMEMGA1UdHwQ8
MDowOKA2oDSGMmh0dHA6Ly9jcmwudGhhd3RlLmNvbS9UaGF3dGVQZXJzb25hbEZyZWVtYWls
Q0EuY3JsMAsGA1UdDwQEAwIBBjApBgNVHREEIjAgpB4wHDEaMBgGA1UEAxMRUHJpdmF0ZUxh
YmVsMi0xMzgwDQYJKoZIhvcNAQEFBQADgYEASIzRUIPqCy7MDaNmrGcPf6+svsIXoUOWlJ1/
TCG4+DYfqi2fNi/A9BxQIJNwPP2t4WFiw9k6GX6EsZkbAMUaC4J0niVQlGLH2ydxVyWN3amc
OY6MIE9lX5Xa9/eH1sYITq726jTlEBpbNU1341YheILcIRk13iSx0x1G/11fZU8xggNkMIID
YAIBATB2MGIxCzAJBgNVBAYTAlpBMSUwIwYDVQQKExxUaGF3dGUgQ29uc3VsdGluZyAoUHR5
KSBMdGQuMSwwKgYDVQQDEyNUaGF3dGUgUGVyc29uYWwgRnJlZW1haWwgSXNzdWluZyBDQQIQ
FbTSUrChYle3zBuZOXUzSTAJBgUrDgMCGgUAoIIBwzAYBgkqhkiG9w0BCQMxCwYJKoZIhvcN
AQcBMBwGCSqGSIb3DQEJBTEPFw0wNjA2MjAxMjMyMzFaMCMGCSqGSIb3DQEJBDEWBBTK70fj
ABOyuEsbK+ECEVlWVSmZeDBSBgkqhkiG9w0BCQ8xRTBDMAoGCCqGSIb3DQMHMA4GCCqGSIb3
DQMCAgIAgDANBggqhkiG9w0DAgIBQDAHBgUrDgMCBzANBggqhkiG9w0DAgIBKDCBhQYJKwYB
BAGCNxAEMXgwdjBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcg
KFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVtYWlsIElzc3Vpbmcg
Q0ECEBW00lKwoWJXt8wbmTl1M0kwgYcGCyqGSIb3DQEJEAILMXigdjBiMQswCQYDVQQGEwJa
QTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhh
d3RlIFBlcnNvbmFsIEZyZWVtYWlsIElzc3VpbmcgQ0ECEBW00lKwoWJXt8wbmTl1M0kwDQYJ
KoZIhvcNAQEBBQAEggEAKf3jfGGPzikwTALM5Ah5hHn9x73dQusaPotqpqCLAle1jkbPW8Wt
kfVfpdub9jqiJUBQkfijv+I4g2LL9doE53SNhBdMWVGVFK1UWo2hvVxiiOlWucIMNz4kvURy
xUo4xDwEz9moexLSrDXb4ToqmTwmh0s7xpJqRI+cUeBMKyauxbRdxRoPJwdykK+Ivwslwihe
THRxh4OMLHRVImOr+tFmA5msxo9zxEr6oklfwvhy93gwr+0niQRLcaS6iRhIeXe8zpr1SU2h
jKquB5hB35NKN17oK2j3LhtkPOzScYNl/1GXtJVu2QPajaGrrPpLS+nu9T1cm4WFkVvW9Re7
5gAAAAAAAA==
--------------ms000505020403050404010907--