xserver: Branch 'master' - 5 commits
Adam Jackson
ajax at kemper.freedesktop.org
Tue Jun 13 14:20:00 UTC 2017
os/connection.c | 154 +++++---------------------------------------------------
os/io.c | 23 +++-----
os/osdep.h | 17 +-----
3 files changed, 29 insertions(+), 165 deletions(-)
New commits:
commit f3689f637f5ac0fb6c231a470e65b39aa5e9ba20
Author: Keith Packard <keithp at keithp.com>
Date: Wed May 17 09:57:29 2017 -0700
os: Set oc->fd to -1 when connection is closed
This ensures that we don't use the now-closed file descriptor in the
future.
Reviewed-by: Adam Jackson <ajax at redhat.com>
Signed-off-by: Keith Packard <keithp at keithp.com>
diff --git a/os/connection.c b/os/connection.c
index 07c16eacf..229bbe745 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -772,6 +772,7 @@ CloseDownFileDescriptor(OsCommPtr oc)
_XSERVTransDisconnect(oc->trans_conn);
_XSERVTransClose(oc->trans_conn);
oc->trans_conn = NULL;
+ oc->fd = -1;
}
}
commit d05c754e1bde895589fb514d8f518afeccecbc05
Author: Keith Packard <keithp at keithp.com>
Date: Wed May 17 09:57:28 2017 -0700
os: Check oc->trans_conn before using oc->fd in YieldControlNoInput
oc->trans_conn is set to NULL when the connection is closed. At this
point, oc->fd is no longer valid and shouldn't be used. Move
dereference of oc->fd up into YieldControlNoInput where the state of
oc->trans_conn can be checked in a single place.
Reviewed-by: Adam Jackson <ajax at redhat.com>
Signed-off-by: Keith Packard <keithp at keithp.com>
diff --git a/os/io.c b/os/io.c
index 6a486613c..b0402912a 100644
--- a/os/io.c
+++ b/os/io.c
@@ -183,10 +183,12 @@ YieldControl(void)
}
static void
-YieldControlNoInput(int fd)
+YieldControlNoInput(ClientPtr client)
{
+ OsCommPtr oc = client->osPrivate;
YieldControl();
- ospoll_reset_events(server_poll, fd);
+ if (oc->trans_conn)
+ ospoll_reset_events(server_poll, oc->fd);
}
static void
@@ -226,7 +228,6 @@ ReadRequestFromClient(ClientPtr client)
{
OsCommPtr oc = (OsCommPtr) client->osPrivate;
ConnectionInputPtr oci = oc->input;
- int fd = oc->fd;
unsigned int gotnow, needed;
int result;
register xReq *request;
@@ -357,7 +358,7 @@ ReadRequestFromClient(ClientPtr client)
if (0)
#endif
{
- YieldControlNoInput(fd);
+ YieldControlNoInput(client);
return 0;
}
}
@@ -394,7 +395,7 @@ ReadRequestFromClient(ClientPtr client)
}
if (gotnow < needed) {
/* Still don't have enough; punt. */
- YieldControlNoInput(fd);
+ YieldControlNoInput(client);
return 0;
}
}
@@ -494,7 +495,6 @@ InsertFakeRequest(ClientPtr client, char *data, int count)
{
OsCommPtr oc = (OsCommPtr) client->osPrivate;
ConnectionInputPtr oci = oc->input;
- int fd = oc->fd;
int gotnow, moveup;
NextAvailableInput(oc);
@@ -533,7 +533,7 @@ InsertFakeRequest(ClientPtr client, char *data, int count)
(gotnow >= (int) (get_req_len((xReq *) oci->bufptr, client) << 2)))
mark_client_ready(client);
else
- YieldControlNoInput(fd);
+ YieldControlNoInput(client);
return TRUE;
}
@@ -548,7 +548,6 @@ ResetCurrentRequest(ClientPtr client)
{
OsCommPtr oc = (OsCommPtr) client->osPrivate;
register ConnectionInputPtr oci = oc->input;
- int fd = oc->fd;
register xReq *request;
int gotnow, needed;
@@ -557,7 +556,7 @@ ResetCurrentRequest(ClientPtr client)
oci->lenLastReq = 0;
gotnow = oci->bufcnt + oci->buffer - oci->bufptr;
if (gotnow < sizeof(xReq)) {
- YieldControlNoInput(fd);
+ YieldControlNoInput(client);
}
else {
request = (xReq *) oci->bufptr;
@@ -576,7 +575,7 @@ ResetCurrentRequest(ClientPtr client)
YieldControl();
}
else
- YieldControlNoInput(fd);
+ YieldControlNoInput(client);
}
}
commit 448a5586e9235bee9648d89e4103ed48e6237c15
Author: Keith Packard <keithp at keithp.com>
Date: Wed May 17 09:57:27 2017 -0700
os: Don't call ospoll_listen/ospoll_mute after connection is closed
In set_poll_client, check oc->trans_conn to make sure the connection
is still running before changing the ospoll configuration of the file
descriptor in case some other bit of the server is now using this file
descriptor.
Reviewed-by: Adam Jackson <ajax at redhat.com>
Signed-off-by: Keith Packard <keithp at keithp.com>
diff --git a/os/connection.c b/os/connection.c
index a45a252a5..07c16eacf 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1084,10 +1084,12 @@ set_poll_client(ClientPtr client)
{
OsCommPtr oc = (OsCommPtr) client->osPrivate;
- if (listen_to_client(client))
- ospoll_listen(server_poll, oc->fd, X_NOTIFY_READ);
- else
- ospoll_mute(server_poll, oc->fd, X_NOTIFY_READ);
+ if (oc->trans_conn) {
+ if (listen_to_client(client))
+ ospoll_listen(server_poll, oc->trans_conn->fd, X_NOTIFY_READ);
+ else
+ ospoll_mute(server_poll, oc->trans_conn->fd, X_NOTIFY_READ);
+ }
}
static void
commit 523d35e3e1c703a655386f6348a4bfb4291c3969
Author: Keith Packard <keithp at keithp.com>
Date: Wed May 17 09:57:26 2017 -0700
os: Use CloseDownFileDescriptor from AbortClient, including ospoll_remove
AbortClient performs most of the same operations as
CloseDownFileDescriptor except that it doesn't call ospoll_remove,
leaving that unaware that the file descriptor has been closed.
If the file descriptor is re-used before the server comes back around
to clean up, and that new file descriptor is passed to SetNotifyFd,
then that function will mistakenly re-interpret the stale ClientPtr
returned by ospoll_data as a struct notify * instead and mangle data
badly.
To fix this, the patch does:
1) Change CloseDownFileDescriptor so that it can be called multiple
times on the same OsCommPtr. The calls related to the file
descriptor are moved inside the check for trans_conn and
oc->trans_conn is set to NULL after cleaning up.
2) Move the XdmcpCloseDisplay call into CloseDownFileDescriptor. I
don't think the actually matters as we just need to know at some
point that the session client has exited. Moving it avoids the
possibility of having this accidentally trigger from another client
with the same fd which closes down at around the same time.
3) Change AbortClient to call CloseDownFileDescriptor. This makes sure
that all of the fd-related clean up happens in the same way
everywhere, in particular ensures that ospoll is notified about the
closed file descriptor at the time it is closed and not some time later.
Debian-bug: https://bugs.debian.org/862824
Reviewed-by: Adam Jackson <ajax at redhat.com>
Signed-off-by: Keith Packard <keithp at keithp.com>
diff --git a/os/connection.c b/os/connection.c
index 66cb61070..a45a252a5 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -757,19 +757,22 @@ ErrorConnMax(XtransConnInfo trans_conn)
/************
* CloseDownFileDescriptor:
- * Remove this file descriptor and it's I/O buffers, etc.
+ * Remove this file descriptor
************/
-static void
+void
CloseDownFileDescriptor(OsCommPtr oc)
{
- int connection = oc->fd;
-
if (oc->trans_conn) {
+ int connection = oc->fd;
+#ifdef XDMCP
+ XdmcpCloseDisplay(connection);
+#endif
+ ospoll_remove(server_poll, connection);
_XSERVTransDisconnect(oc->trans_conn);
_XSERVTransClose(oc->trans_conn);
+ oc->trans_conn = NULL;
}
- ospoll_remove(server_poll, connection);
}
/*****************
@@ -787,9 +790,6 @@ CloseDownConnection(ClientPtr client)
if (oc->output)
FlushClient(client, oc, (char *) NULL, 0);
-#ifdef XDMCP
- XdmcpCloseDisplay(oc->fd);
-#endif
CloseDownFileDescriptor(oc);
FreeOsBuffers(oc);
free(client->osPrivate);
diff --git a/os/io.c b/os/io.c
index e1b90d17e..6a486613c 100644
--- a/os/io.c
+++ b/os/io.c
@@ -647,9 +647,7 @@ AbortClient(ClientPtr client)
OsCommPtr oc = client->osPrivate;
if (oc->trans_conn) {
- _XSERVTransDisconnect(oc->trans_conn);
- _XSERVTransClose(oc->trans_conn);
- oc->trans_conn = NULL;
+ CloseDownFileDescriptor(oc);
mark_client_ready(client);
}
}
diff --git a/os/osdep.h b/os/osdep.h
index c98b9a2a6..3ab81958c 100644
--- a/os/osdep.h
+++ b/os/osdep.h
@@ -133,6 +133,9 @@ extern int FlushClient(ClientPtr /*who */ ,
extern void FreeOsBuffers(OsCommPtr /*oc */
);
+void
+CloseDownFileDescriptor(OsCommPtr oc);
+
#include "dix.h"
#include "ospoll.h"
commit 5d941ccb0b30399d505b48bff894c95cc3023bbe
Author: Keith Packard <keithp at keithp.com>
Date: Wed May 17 09:57:25 2017 -0700
os: Eliminate ConnectionTranslation
This infrastructure is no longer read, only written; the mapping
from fd to client is now handled by ospoll.
Reviewed-by: Adam Jackson <ajax at redhat.com>
Signed-off-by: Keith Packard <keithp at keithp.com>
diff --git a/os/connection.c b/os/connection.c
index 62e298072..66cb61070 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -142,96 +142,6 @@ set_poll_client(ClientPtr client);
static void
set_poll_clients(void);
-#if !defined(WIN32)
-int *ConnectionTranslation = NULL;
-int ConnectionTranslationSize = 0;
-#else
-/*
- * On NT fds are not small integers, they are unrelated, and there is
- * not even a known maximum value, so use something quite arbitrary for now.
- * Do storage is a hash table of size 256. Collisions are handled in a linked
- * list.
- */
-
-struct _ct_node {
- struct _ct_node *next;
- int key;
- int value;
-};
-
-struct _ct_node *ct_head[256];
-
-void
-InitConnectionTranslation(void)
-{
- memset(ct_head, 0, sizeof(ct_head));
-}
-
-int
-GetConnectionTranslation(int conn)
-{
- struct _ct_node *node = ct_head[conn & 0xff];
-
- while (node != NULL) {
- if (node->key == conn)
- return node->value;
- node = node->next;
- }
- return 0;
-}
-
-void
-SetConnectionTranslation(int conn, int client)
-{
- struct _ct_node **node = ct_head + (conn & 0xff);
-
- if (client == 0) { /* remove entry */
- while (*node != NULL) {
- if ((*node)->key == conn) {
- struct _ct_node *temp = *node;
-
- *node = (*node)->next;
- free(temp);
- return;
- }
- node = &((*node)->next);
- }
- return;
- }
- else {
- while (*node != NULL) {
- if ((*node)->key == conn) {
- (*node)->value = client;
- return;
- }
- node = &((*node)->next);
- }
- *node = malloc(sizeof(struct _ct_node));
- (*node)->next = NULL;
- (*node)->key = conn;
- (*node)->value = client;
- return;
- }
-}
-
-void
-ClearConnectionTranslation(void)
-{
- unsigned i;
-
- for (i = 0; i < 256; i++) {
- struct _ct_node *node = ct_head[i];
-
- while (node != NULL) {
- struct _ct_node *temp = node;
-
- node = node->next;
- free(temp);
- }
- }
-}
-#endif
-
static XtransConnInfo *ListenTransConns = NULL;
static int *ListenTransFds = NULL;
static int ListenTransCount;
@@ -252,7 +162,7 @@ lookup_trans_conn(int fd)
return NULL;
}
-/* Set MaxClients and lastfdesc, and allocate ConnectionTranslation */
+/* Set MaxClients */
void
InitConnectionLimits(void)
@@ -262,15 +172,6 @@ InitConnectionLimits(void)
#ifdef DEBUG
ErrorF("InitConnectionLimits: MaxClients = %d\n", MaxClients);
#endif
-
-#if !defined(WIN32)
- if (!ConnectionTranslation) {
- ConnectionTranslation = xnfallocarray(MaxClients, sizeof(int));
- ConnectionTranslationSize = MaxClients;
- }
-#else
- InitConnectionTranslation();
-#endif
}
/*
@@ -345,13 +246,6 @@ CreateWellKnownSockets(void)
int i;
int partial;
-#if !defined(WIN32)
- for (i = 0; i < ConnectionTranslationSize; i++)
- ConnectionTranslation[i] = 0;
-#else
- ClearConnectionTranslation();
-#endif
-
/* display is initialized to "0" by main(). It is then set to the display
* number if specified on the command line. */
@@ -737,15 +631,6 @@ AllocNewConnection(XtransConnInfo trans_conn, int fd, CARD32 conn_time)
return NullClient;
}
client->local = ComputeLocalClient(client);
-#if !defined(WIN32)
- if (fd >= ConnectionTranslationSize) {
- ConnectionTranslationSize *= 2;
- ConnectionTranslation = xnfreallocarray(ConnectionTranslation, ConnectionTranslationSize, sizeof (int));
- }
- ConnectionTranslation[fd] = client->index;
-#else
- SetConnectionTranslation(fd, client->index);
-#endif
ospoll_add(server_poll, fd,
ospoll_trigger_edge,
ClientReady,
@@ -781,7 +666,6 @@ EstablishNewConnections(ClientPtr clientUnused, void *closure)
OsCommPtr oc;
XtransConnInfo trans_conn, new_trans_conn;
int status;
- int clientid;
connect_time = GetTimeInMillis();
/* kill off stragglers */
@@ -803,10 +687,6 @@ EstablishNewConnections(ClientPtr clientUnused, void *closure)
newconn = _XSERVTransGetConnectionNumber(new_trans_conn);
- clientid = GetConnectionTranslation(newconn);
- if (clientid && (client = clients[clientid]))
- CloseDownClient(client);
-
_XSERVTransSetOption(new_trans_conn, TRANS_NONBLOCKING, 1);
if (trans_conn->flags & TRANS_NOXAUTH)
@@ -889,11 +769,6 @@ CloseDownFileDescriptor(OsCommPtr oc)
_XSERVTransDisconnect(oc->trans_conn);
_XSERVTransClose(oc->trans_conn);
}
-#ifndef WIN32
- ConnectionTranslation[connection] = 0;
-#else
- SetConnectionTranslation(connection, 0);
-#endif
ospoll_remove(server_poll, connection);
}
diff --git a/os/osdep.h b/os/osdep.h
index c5bec3f56..c98b9a2a6 100644
--- a/os/osdep.h
+++ b/os/osdep.h
@@ -141,20 +141,6 @@ extern struct ospoll *server_poll;
Bool
listen_to_client(ClientPtr client);
-#if !defined(WIN32) || defined(__CYGWIN__)
-extern int *ConnectionTranslation;
-extern int ConnectionTranslationSize;
-static inline int GetConnectionTranslation(int conn) {
- if (conn >= ConnectionTranslationSize)
- return 0;
- return ConnectionTranslation[conn];
-}
-#else
-extern int GetConnectionTranslation(int conn);
-extern void SetConnectionTranslation(int conn, int client);
-extern void ClearConnectionTranslation(void);
-#endif
-
extern Bool NewOutputPending;
extern WorkQueuePtr workQueue;
More information about the xorg-commit
mailing list