[PATCH RFC xserver] os: Add a mutex to protect io buffers
Olivier Fourdan
ofourdan at redhat.com
Wed Feb 22 15:50:22 UTC 2017
WriteToClient() can be called from XIChangeDeviceProperty() so from the
InputThread which is a problem as it allocates and free the input and
output buffers.
Add a new mutex to protect accesses to the input and output buffers from
being accessed from different threads.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99164
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99887
Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
---
RFC: This is probably sub-optimal and broken in many places, but it
seems to avoid the memory corruption (so far)... At least it's a
start, I guess.
os/io.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 66 insertions(+), 7 deletions(-)
diff --git a/os/io.c b/os/io.c
index be85226..b0e3825 100644
--- a/os/io.c
+++ b/os/io.c
@@ -107,6 +107,36 @@ static ConnectionInputPtr FreeInputs = (ConnectionInputPtr) NULL;
static ConnectionOutputPtr FreeOutputs = (ConnectionOutputPtr) NULL;
static OsCommPtr AvailableInput = (OsCommPtr) NULL;
+/* iobuf_mutex code copied from inputthread.c */
+#ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
+static pthread_mutex_t iobuf_mutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+#else
+static pthread_mutex_t iobuf_mutex;
+static Bool iobuf_mutex_initialized;
+#endif
+
+static void
+iobuf_lock(void)
+{
+#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
+ if (!iobuf_mutex_initialized) {
+ pthread_mutexattr_t mutex_attr;
+
+ iobuf_mutex_initialized = TRUE;
+ pthread_mutexattr_init(&mutex_attr);
+ pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE);
+ pthread_mutex_init(&iobuf_mutex, &mutex_attr);
+ }
+#endif
+ pthread_mutex_lock(&iobuf_mutex);
+}
+
+static void
+iobuf_unlock(void)
+{
+ pthread_mutex_unlock(&iobuf_mutex);
+}
+
#define get_req_len(req,cli) ((cli)->swapped ? \
lswaps((req)->length) : (req)->length)
@@ -203,6 +233,7 @@ YieldControlDeath(void)
static void
NextAvailableInput(OsCommPtr oc)
{
+ iobuf_lock();
if (AvailableInput) {
if (AvailableInput != oc) {
ConnectionInputPtr aci = AvailableInput->input;
@@ -219,6 +250,7 @@ NextAvailableInput(OsCommPtr oc)
}
AvailableInput = NULL;
}
+ iobuf_unlock();
}
int
@@ -237,12 +269,14 @@ ReadRequestFromClient(ClientPtr client)
/* make sure we have an input buffer */
+ iobuf_lock();
if (!oci) {
if ((oci = FreeInputs)) {
FreeInputs = oci->next;
}
else if (!(oci = AllocateInputBuffer())) {
YieldControlDeath();
+ iobuf_unlock();
return -1;
}
oc->input = oci;
@@ -313,6 +347,7 @@ ReadRequestFromClient(ClientPtr client)
*/
oci->ignoreBytes = needed - gotnow;
oci->lenLastReq = gotnow;
+ iobuf_unlock();
return needed;
}
if ((gotnow == 0) || ((oci->bufptr - oci->buffer + needed) > oci->size)) {
@@ -346,6 +381,7 @@ ReadRequestFromClient(ClientPtr client)
* used to happen
*/
YieldControlDeath();
+ iobuf_unlock();
return -1;
}
result = _XSERVTransRead(oc->trans_conn, oci->buffer + oci->bufcnt,
@@ -358,10 +394,12 @@ ReadRequestFromClient(ClientPtr client)
#endif
{
YieldControlNoInput(fd);
+ iobuf_unlock();
return 0;
}
}
YieldControlDeath();
+ iobuf_unlock();
return -1;
}
oci->bufcnt += result;
@@ -395,6 +433,7 @@ ReadRequestFromClient(ClientPtr client)
if (gotnow < needed) {
/* Still don't have enough; punt. */
YieldControlNoInput(fd);
+ iobuf_unlock();
return 0;
}
}
@@ -447,6 +486,8 @@ ReadRequestFromClient(ClientPtr client)
client->req_len -= bytes_to_int32(sizeof(xBigReq) - sizeof(xReq));
}
client->requestBuffer = (void *) oci->bufptr;
+ iobuf_unlock();
+
#ifdef DEBUG_COMMUNICATION
{
xReq *req = client->requestBuffer;
@@ -498,12 +539,14 @@ InsertFakeRequest(ClientPtr client, char *data, int count)
int gotnow, moveup;
NextAvailableInput(oc);
-
+ iobuf_lock();
if (!oci) {
if ((oci = FreeInputs))
FreeInputs = oci->next;
- else if (!(oci = AllocateInputBuffer()))
+ else if (!(oci = AllocateInputBuffer())) {
+ iobuf_unlock();
return FALSE;
+ }
oc->input = oci;
}
oci->bufptr += oci->lenLastReq;
@@ -513,8 +556,10 @@ InsertFakeRequest(ClientPtr client, char *data, int count)
char *ibuf;
ibuf = (char *) realloc(oci->buffer, gotnow + count);
- if (!ibuf)
+ if (!ibuf) {
+ iobuf_unlock();
return FALSE;
+ }
oci->size = gotnow + count;
oci->buffer = ibuf;
oci->bufptr = ibuf + oci->bufcnt - gotnow;
@@ -534,6 +579,7 @@ InsertFakeRequest(ClientPtr client, char *data, int count)
mark_client_ready(client);
else
YieldControlNoInput(fd);
+ iobuf_unlock();
return TRUE;
}
@@ -698,7 +744,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
multicount = TRUE;
}
#endif
-
+ iobuf_lock();
if (!oco) {
if ((oco = FreeOutputs)) {
FreeOutputs = oco->next;
@@ -710,6 +756,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
oc->trans_conn = NULL;
}
MarkClientException(who);
+ iobuf_unlock();
return -1;
}
oc->output = oco;
@@ -763,7 +810,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
CriticalOutputPending = FALSE;
NewOutputPending = FALSE;
}
-
+ iobuf_unlock();
return FlushClient(who, oc, buf, count);
}
@@ -775,6 +822,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
memset(oco->buf + oco->count, '\0', padBytes);
oco->count += padBytes;
}
+ iobuf_unlock();
return count;
}
@@ -805,10 +853,12 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount)
return 0;
written = 0;
padsize = padding_for_int32(extraCount);
+ iobuf_lock();
notWritten = oco->count + extraCount + padsize;
- if (!notWritten)
+ if (!notWritten) {
+ iobuf_unlock();
return 0;
-
+ }
if (FlushCallback)
CallCallbacks(&FlushCallback, who);
@@ -894,6 +944,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount)
oc->trans_conn = NULL;
MarkClientException(who);
oco->count = 0;
+ iobuf_unlock();
return -1;
}
oco->size = notWritten + BUFSIZE;
@@ -908,6 +959,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount)
oco->count = notWritten; /* this will include the pad */
ospoll_listen(server_poll, oc->fd, X_NOTIFY_WRITE);
+ iobuf_unlock();
/* return only the amount explicitly requested */
return extraCount;
@@ -925,6 +977,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount)
}
MarkClientException(who);
oco->count = 0;
+ iobuf_unlock();
return -1;
}
}
@@ -942,6 +995,8 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void *__extraBuf, int extraCount)
FreeOutputs = oco;
}
oc->output = (ConnectionOutputPtr) NULL;
+ iobuf_unlock();
+
return extraCount; /* return only the amount explicitly requested */
}
@@ -990,6 +1045,7 @@ FreeOsBuffers(OsCommPtr oc)
ConnectionInputPtr oci;
ConnectionOutputPtr oco;
+ iobuf_lock();
if (AvailableInput == oc)
AvailableInput = (OsCommPtr) NULL;
if ((oci = oc->input)) {
@@ -1017,6 +1073,7 @@ FreeOsBuffers(OsCommPtr oc)
oco->count = 0;
}
}
+ iobuf_unlock();
}
void
@@ -1025,6 +1082,7 @@ ResetOsBuffers(void)
ConnectionInputPtr oci;
ConnectionOutputPtr oco;
+ iobuf_lock();
while ((oci = FreeInputs)) {
FreeInputs = oci->next;
free(oci->buffer);
@@ -1035,4 +1093,5 @@ ResetOsBuffers(void)
free(oco->buf);
free(oco);
}
+ iobuf_unlock();
}
--
2.9.3
More information about the xorg-devel
mailing list