From b15aa6bd4efde784e546d168bb23b8a8e816e85b Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Wed, 22 Apr 2015 09:23:47 +0200 Subject: [PATCH] Add xcb_send_request_with_fds() and *_with_fds64() Doing xcb_send_fd(), xcb_send_request() is racy. If two threads do this at the same time, they could mix up their file descriptors. This commit makes it possibly to fix this race by providing a single function which does everything that is needed. Signed-off-by: Uli Schlachter --- src/xcb_out.c | 22 +++++++++++++-- src/xcbext.h | 77 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/src/xcb_out.c b/src/xcb_out.c index ec132fc..b5b6f34 100644 --- a/src/xcb_out.c +++ b/src/xcb_out.c @@ -214,15 +214,18 @@ static void send_fds(xcb_connection_t *c, int *fds, unsigned int num_fds) close_fds(fds, num_fds); } -uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req) +uint64_t xcb_send_request_with_fds64(xcb_connection_t *c, int flags, struct iovec *vector, + const xcb_protocol_request_t *req, unsigned int num_fds, int *fds) { uint64_t request; uint32_t prefix[2]; int veclen = req->count; enum workarounds workaround = WORKAROUND_NONE; - if(c->has_error) + if(c->has_error) { + close_fds(fds, num_fds); return 0; + } assert(c != 0); assert(vector != 0); @@ -241,6 +244,7 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector const xcb_query_extension_reply_t *extension = xcb_get_extension_data(c, req->ext); if(!(extension && extension->present)) { + close_fds(fds, num_fds); _xcb_conn_shutdown(c, XCB_CONN_CLOSED_EXT_NOTSUPPORTED); return 0; } @@ -271,6 +275,7 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector } else if(longlen > xcb_get_maximum_request_length(c)) { + close_fds(fds, num_fds); _xcb_conn_shutdown(c, XCB_CONN_CLOSED_REQ_LEN_EXCEED); return 0; /* server can't take this; maybe need BIGREQUESTS? */ } @@ -317,12 +322,25 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector prepare_socket_request(c); } + send_fds(c, fds, num_fds); send_request(c, req->isvoid, workaround, flags, vector, veclen); request = c->has_error ? 0 : c->out.request; pthread_mutex_unlock(&c->iolock); return request; } +/* request number are actually uint64_t internally but keep API compat with unsigned int */ +unsigned int xcb_send_request_with_fds(xcb_connection_t *c, int flags, struct iovec *vector, + const xcb_protocol_request_t *req, unsigned int num_fds, int *fds) +{ + return xcb_send_request_with_fds64(c, flags, vector, req, num_fds, fds); +} + +uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req) +{ + return xcb_send_request_with_fds64(c, flags, vector, req, 0, NULL); +} + /* request number are actually uint64_t internally but keep API compat with unsigned int */ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req) { diff --git a/src/xcbext.h b/src/xcbext.h index b2575f7..44d789e 100644 --- a/src/xcbext.h +++ b/src/xcbext.h @@ -68,13 +68,13 @@ enum xcb_send_request_flags_t { * * This function sends a new request to the X server. The data of the request is * given as an array of @c iovecs in the @p vector argument. The length of that - * array and the neccessary management information are given in the @p request + * array and the necessary management information are given in the @p request * argument. * * When this function returns, the request might or might not be sent already. * Use xcb_flush() to make sure that it really was sent. * - * Please note that this function is not the prefered way for sending requests. + * Please note that this function is not the preferred way for sending requests. * It's better to use the generated wrapper functions. * * Please note that xcb might use index -1 and -2 of the @p vector array internally, @@ -82,6 +82,37 @@ enum xcb_send_request_flags_t { */ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request); +/** + * @brief Send a request to the server. + * @param c: The connection to the X server. + * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration. + * @param vector: Data to send; must have two iovecs before start for internal use. + * @param request: Information about the request to be sent. + * @param num_fds: Number of additional file descriptors to send to the server + * @param fds: Additional file descriptors that should be send to the server. + * @return The request's sequence number on success, 0 otherwise. + * + * This function sends a new request to the X server. The data of the request is + * given as an array of @c iovecs in the @p vector argument. The length of that + * array and the necessary management information are given in the @p request + * argument. + * + * If @p num_fds is non-zero, @p fds points to an array of file descriptors that + * will be sent to the X server along with this request. After this function + * returns, all file descriptors sent are owned by xcb and will be closed + * eventually. + * + * When this function returns, the request might or might not be sent already. + * Use xcb_flush() to make sure that it really was sent. + * + * Please note that this function is not the preferred way for sending requests. + * + * Please note that xcb might use index -1 and -2 of the @p vector array internally, + * so they must be valid! + */ +unsigned int xcb_send_request_with_fds(xcb_connection_t *c, int flags, struct iovec *vector, + const xcb_protocol_request_t *request, unsigned int num_fds, int *fds); + /** * @brief Send a request to the server, with 64-bit sequence number returned. * @param c: The connection to the X server. @@ -92,13 +123,13 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect * * This function sends a new request to the X server. The data of the request is * given as an array of @c iovecs in the @p vector argument. The length of that - * array and the neccessary management information are given in the @p request + * array and the necessary management information are given in the @p request * argument. * * When this function returns, the request might or might not be sent already. * Use xcb_flush() to make sure that it really was sent. * - * Please note that this function is not the prefered way for sending requests. + * Please note that this function is not the preferred way for sending requests. * It's better to use the generated wrapper functions. * * Please note that xcb might use index -1 and -2 of the @p vector array internally, @@ -106,6 +137,38 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect */ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request); +/** + * @brief Send a request to the server, with 64-bit sequence number returned. + * @param c: The connection to the X server. + * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration. + * @param vector: Data to send; must have two iovecs before start for internal use. + * @param request: Information about the request to be sent. + * @param num_fds: Number of additional file descriptors to send to the server + * @param fds: Additional file descriptors that should be send to the server. + * @return The request's sequence number on success, 0 otherwise. + * + * This function sends a new request to the X server. The data of the request is + * given as an array of @c iovecs in the @p vector argument. The length of that + * array and the necessary management information are given in the @p request + * argument. + * + * If @p num_fds is non-zero, @p fds points to an array of file descriptors that + * will be sent to the X server along with this request. After this function + * returns, all file descriptors sent are owned by xcb and will be closed + * eventually. + * + * When this function returns, the request might or might not be sent already. + * Use xcb_flush() to make sure that it really was sent. + * + * Please note that this function is not the preferred way for sending requests. + * It's better to use the generated wrapper functions. + * + * Please note that xcb might use index -1 and -2 of the @p vector array internally, + * so they must be valid! + */ +uint64_t xcb_send_request_with_fds64(xcb_connection_t *c, int flags, struct iovec *vector, + const xcb_protocol_request_t *request, unsigned int num_fds, int *fds); + /** * @brief Send a file descriptor to the server in the next call to xcb_send_request. * @param c: The connection to the X server. @@ -114,9 +177,9 @@ uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector * After this function returns, the file descriptor given is owned by xcb and * will be closed eventually. * - * FIXME: How the heck is this supposed to work in a thread-safe way? There is a - * race between two threads doing xcb_send_fd(); xcb_send_request(); at the same - * time. + * @deprecated This function cannot be used in a thread-safe way. Two threads + * that run xcb_send_fd(); xcb_send_request(); could mix up their file + * descriptors. Instead, xcb_send_request_with_fds() should be used. */ void xcb_send_fd(xcb_connection_t *c, int fd);