|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] Avoid race when checking for an active transaction
From: Paul Durrant <pdurrant@xxxxxxxxxx>
The code in StoreTransactionEnd() checks, under the protection of Context->Lock,
that the transaction is active. It then drops the lock and calls
StorePrepareRequest(), which also tests whether the transaction is active but
*not* under the protection of the lock. If the domain is suspended and resumed
in between the two checks then this will cause StorePrepareRequest() to fail
for non-NULL transactions.
This patch makes sure that Context->Lock is held across all calls to
StorePrepareRequest(), along with any prior tests for whether transactions or
watches are active, and drops the internal acquisition of the lock (which was
done to protect the increment of Context->RequestId).
Reported-by: Owen Smith <owen.smith@xxxxxxxxxx>
Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
---
src/xenbus/store.c | 64 +++++++++++++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 15 deletions(-)
diff --git a/src/xenbus/store.c b/src/xenbus/store.c
index 5ffea1fe4859..028fd59f07b6 100644
--- a/src/xenbus/store.c
+++ b/src/xenbus/store.c
@@ -180,7 +180,6 @@ StorePrepareRequest(
)
{
ULONG Id;
- KIRQL Irql;
PXENBUS_STORE_SEGMENT Segment;
va_list Arguments;
NTSTATUS status;
@@ -200,10 +199,7 @@ StorePrepareRequest(
Request->Header.type = Type;
Request->Header.tx_id = Id;
Request->Header.len = 0;
-
- KeAcquireSpinLock(&Context->Lock, &Irql);
Request->Header.req_id = Context->RequestId++;
- KeReleaseSpinLock(&Context->Lock, Irql);
Request->Count = 0;
Segment = &Request->Segment[Request->Count++];
@@ -1102,6 +1098,7 @@ StoreRead(
PXENBUS_STORE_CONTEXT Context = Interface->Context;
PVOID Caller;
XENBUS_STORE_REQUEST Request;
+ KIRQL Irql;
PXENBUS_STORE_RESPONSE Response;
PXENBUS_STORE_BUFFER Buffer;
NTSTATUS status;
@@ -1110,6 +1107,8 @@ StoreRead(
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
if (Prefix == NULL) {
status = StorePrepareRequest(Context,
&Request,
@@ -1130,6 +1129,8 @@ StoreRead(
NULL, 0);
}
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
if (!NT_SUCCESS(status))
goto fail1;
@@ -1177,11 +1178,14 @@ StoreWrite(
)
{
XENBUS_STORE_REQUEST Request;
+ KIRQL Irql;
PXENBUS_STORE_RESPONSE Response;
NTSTATUS status;
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
if (Prefix == NULL) {
status = StorePrepareRequest(Context,
&Request,
@@ -1204,6 +1208,8 @@ StoreWrite(
NULL, 0);
}
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
if (!NT_SUCCESS(status))
goto fail1;
@@ -1326,11 +1332,14 @@ StoreRemove(
{
PXENBUS_STORE_CONTEXT Context = Interface->Context;
XENBUS_STORE_REQUEST Request;
+ KIRQL Irql;
PXENBUS_STORE_RESPONSE Response;
NTSTATUS status;
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
if (Prefix == NULL) {
status = StorePrepareRequest(Context,
&Request,
@@ -1351,6 +1360,8 @@ StoreRemove(
NULL, 0);
}
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
if (!NT_SUCCESS(status))
goto fail1;
@@ -1391,6 +1402,7 @@ StoreDirectory(
PXENBUS_STORE_CONTEXT Context = Interface->Context;
PVOID Caller;
XENBUS_STORE_REQUEST Request;
+ KIRQL Irql;
PXENBUS_STORE_RESPONSE Response;
PXENBUS_STORE_BUFFER Buffer;
NTSTATUS status;
@@ -1399,6 +1411,8 @@ StoreDirectory(
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
if (Prefix == NULL) {
status = StorePrepareRequest(Context,
&Request,
@@ -1419,6 +1433,8 @@ StoreDirectory(
NULL, 0);
}
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
if (!NT_SUCCESS(status))
goto fail1;
@@ -1486,12 +1502,17 @@ StoreTransactionStart(
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
status = StorePrepareRequest(Context,
&Request,
NULL,
XS_TRANSACTION_START,
"", 1,
NULL, 0);
+
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
ASSERT(NT_SUCCESS(status));
Response = StoreSubmitRequest(Context, &Request);
@@ -1555,22 +1576,23 @@ StoreTransactionEnd(
ASSERT3U(Transaction->Magic, ==, STORE_TRANSACTION_MAGIC);
+ RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+
KeAcquireSpinLock(&Context->Lock, &Irql);
status = STATUS_RETRY;
if (!Transaction->Active)
goto done;
- KeReleaseSpinLock(&Context->Lock, Irql);
-
- RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
-
status = StorePrepareRequest(Context,
&Request,
Transaction,
XS_TRANSACTION_END,
(Commit) ? "T" : "F", 2,
NULL, 0);
+
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
ASSERT(NT_SUCCESS(status));
Response = StoreSubmitRequest(Context, &Request);
@@ -1678,6 +1700,8 @@ StoreWatchAdd(
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
status = StorePrepareRequest(Context,
&Request,
NULL,
@@ -1687,6 +1711,9 @@ StoreWatchAdd(
Token, strlen(Token),
"", 1,
NULL, 0);
+
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
ASSERT(NT_SUCCESS(status));
Response = StoreSubmitRequest(Context, &Request);
@@ -1760,13 +1787,6 @@ StoreWatchRemove(
Path = Watch->Path;
- KeAcquireSpinLock(&Context->Lock, &Irql);
-
- if (!Watch->Active)
- goto done;
-
- KeReleaseSpinLock(&Context->Lock, Irql);
-
status = RtlStringCbPrintfA(Token,
sizeof (Token),
"TOK|%p|%04X",
@@ -1777,6 +1797,11 @@ StoreWatchRemove(
RtlZeroMemory(&Request, sizeof (XENBUS_STORE_REQUEST));
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
+ if (!Watch->Active)
+ goto done;
+
status = StorePrepareRequest(Context,
&Request,
NULL,
@@ -1786,6 +1811,9 @@ StoreWatchRemove(
Token, strlen(Token),
"", 1,
NULL, 0);
+
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
ASSERT(NT_SUCCESS(status));
Response = StoreSubmitRequest(Context, &Request);
@@ -2002,6 +2030,7 @@ StorePermissionsSet(
{
PXENBUS_STORE_CONTEXT Context = Interface->Context;
XENBUS_STORE_REQUEST Request;
+ KIRQL Irql;
PXENBUS_STORE_RESPONSE Response;
NTSTATUS status;
ULONG Index;
@@ -2048,6 +2077,8 @@ StorePermissionsSet(
Length -= Used;
}
+ KeAcquireSpinLock(&Context->Lock, &Irql);
+
status = StorePrepareRequest(Context,
&Request,
Transaction,
@@ -2056,6 +2087,9 @@ StorePermissionsSet(
"", 1,
PermissionString, XENSTORE_PAYLOAD_MAX -
Length,
NULL, 0);
+
+ KeReleaseSpinLock(&Context->Lock, Irql);
+
if (!NT_SUCCESS(status))
goto fail4;
--
2.25.1
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |