diff options
| author | Keith Packard <keithp@keithp.com> | 2017-05-17 09:57:26 -0700 |
|---|---|---|
| committer | Adam Jackson <ajax@redhat.com> | 2017-06-13 09:56:34 -0400 |
| commit | 523d35e3e1c703a655386f6348a4bfb4291c3969 (patch) | |
| tree | ae4790db56c53d9181946dec156021985eece441 /os | |
| parent | 5d941ccb0b30399d505b48bff894c95cc3023bbe (diff) | |
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@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
Diffstat (limited to 'os')
| -rw-r--r-- | os/connection.c | 16 | ||||
| -rw-r--r-- | os/io.c | 4 | ||||
| -rw-r--r-- | os/osdep.h | 3 |
3 files changed, 12 insertions, 11 deletions
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); @@ -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" |