From aa4deef8a86ccddbd0d9a7d894e72ac329d8960d Mon Sep 17 00:00:00 2001 From: Ashkan Kiani Date: Tue, 16 Apr 2019 17:35:39 -0700 Subject: Fix the payload type returned by IPC If a client is subscribed and sends a subsequent ipc command which causes event updates, then those event updates override the `client->current_command` and send the incorrect type for the payload associated with the command. Example: SUBSCRIBE {window} RUN_COMMAND focus -> PAYLOAD_TYPE is 0x80000002 for window events Therefore, we decouple the `client->current_command` by passing it as an argument to the ipc_send_reply function, avoiding a data race. The same is done for the `client->payload_length` as a precautionary measure for the same reason. --- sway/ipc-server.c | 136 +++++++++++++++++++++++++++++------------------------- 1 file changed, 72 insertions(+), 64 deletions(-) diff --git a/sway/ipc-server.c b/sway/ipc-server.c index e133a5bf..ca1c1b12 100644 --- a/sway/ipc-server.c +++ b/sway/ipc-server.c @@ -47,13 +47,14 @@ struct ipc_client { struct wl_event_source *writable_event_source; struct sway_server *server; int fd; - uint32_t payload_length; uint32_t security_policy; - enum ipc_command_type current_command; enum ipc_command_type subscribed_events; size_t write_buffer_len; size_t write_buffer_size; char *write_buffer; + // The following are for storing data between event_loop calls + uint32_t pending_length; + enum ipc_command_type pending_type; }; struct sockaddr_un *ipc_user_sockaddr(void); @@ -61,8 +62,10 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data); int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data); int ipc_client_handle_writable(int client_fd, uint32_t mask, void *data); void ipc_client_disconnect(struct ipc_client *client); -void ipc_client_handle_command(struct ipc_client *client); -bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t payload_length); +void ipc_client_handle_command(struct ipc_client *client, uint32_t payload_length, + enum ipc_command_type payload_type); +bool ipc_send_reply(struct ipc_client *client, enum ipc_command_type payload_type, + const char *payload, uint32_t payload_length); static void handle_display_destroy(struct wl_listener *listener, void *data) { if (ipc_event_source) { @@ -178,7 +181,7 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) { return 0; } client->server = server; - client->payload_length = 0; + client->pending_length = 0; client->fd = client_fd; client->subscribed_events = 0; client->event_source = wl_event_loop_add_fd(server->wl_event_loop, @@ -224,9 +227,13 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) { } // Wait for the rest of the command payload in case the header has already been read - if (client->payload_length > 0) { - if ((uint32_t)read_available >= client->payload_length) { - ipc_client_handle_command(client); + if (client->pending_length > 0) { + if ((uint32_t)read_available >= client->pending_length) { + // Reset pending values. + uint32_t pending_length = client->pending_length; + enum ipc_command_type pending_type = client->pending_type; + client->pending_length = 0; + ipc_client_handle_command(client, pending_length, pending_type); } return 0; } @@ -251,11 +258,15 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) { return 0; } - memcpy(&client->payload_length, &buf32[0], sizeof(buf32[0])); - memcpy(&client->current_command, &buf32[1], sizeof(buf32[1])); + memcpy(&client->pending_length, &buf32[0], sizeof(buf32[0])); + memcpy(&client->pending_type, &buf32[1], sizeof(buf32[1])); - if (read_available - received >= (long)client->payload_length) { - ipc_client_handle_command(client); + if (read_available - received >= (long)client->pending_length) { + // Reset pending values. + uint32_t pending_length = client->pending_length; + enum ipc_command_type pending_type = client->pending_type; + client->pending_length = 0; + ipc_client_handle_command(client, pending_length, pending_type); } return 0; @@ -278,8 +289,8 @@ static void ipc_send_event(const char *json_string, enum ipc_command_type event) if ((client->subscribed_events & event_mask(event)) == 0) { continue; } - client->current_command = event; - if (!ipc_send_reply(client, json_string, (uint32_t) strlen(json_string))) { + if (!ipc_send_reply(client, event, json_string, + (uint32_t)strlen(json_string))) { sway_log_errno(SWAY_INFO, "Unable to send reply to IPC client"); /* ipc_send_reply destroys client on error, which also * removes it from the list, so we need to process @@ -567,20 +578,21 @@ static void ipc_get_marks_callback(struct sway_container *con, void *data) { } } -void ipc_client_handle_command(struct ipc_client *client) { +void ipc_client_handle_command(struct ipc_client *client, uint32_t payload_length, + enum ipc_command_type payload_type) { if (!sway_assert(client != NULL, "client != NULL")) { return; } - char *buf = malloc(client->payload_length + 1); + char *buf = malloc(payload_length + 1); if (!buf) { sway_log_errno(SWAY_INFO, "Unable to allocate IPC payload"); ipc_client_disconnect(client); return; } - if (client->payload_length > 0) { + if (payload_length > 0) { // Payload should be fully available - ssize_t received = recv(client->fd, buf, client->payload_length, 0); + ssize_t received = recv(client->fd, buf, payload_length, 0); if (received == -1) { sway_log_errno(SWAY_INFO, "Unable to receive payload from IPC client"); @@ -589,16 +601,15 @@ void ipc_client_handle_command(struct ipc_client *client) { return; } } - buf[client->payload_length] = '\0'; + buf[payload_length] = '\0'; - bool client_valid = true; - switch (client->current_command) { + switch (payload_type) { case IPC_COMMAND: { char *line = strtok(buf, "\n"); while (line) { size_t line_length = strlen(line); - if (line + line_length >= buf + client->payload_length) { + if (line + line_length >= buf + payload_length) { break; } line[line_length] = ';'; @@ -609,7 +620,7 @@ void ipc_client_handle_command(struct ipc_client *client) { transaction_commit_dirty(); char *json = cmd_results_to_json(res_list); int length = strlen(json); - client_valid = ipc_send_reply(client, json, (uint32_t)length); + ipc_send_reply(client, payload_type, json, (uint32_t)length); free(json); while (res_list->length) { struct cmd_results *results = res_list->items[0]; @@ -623,7 +634,7 @@ void ipc_client_handle_command(struct ipc_client *client) { case IPC_SEND_TICK: { ipc_event_tick(buf); - ipc_send_reply(client, "{\"success\": true}", 17); + ipc_send_reply(client, payload_type, "{\"success\": true}", 17); goto exit_cleanup; } @@ -656,8 +667,8 @@ void ipc_client_handle_command(struct ipc_client *client) { } } const char *json_string = json_object_to_json_string(outputs); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(outputs); // free goto exit_cleanup; } @@ -667,8 +678,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object *workspaces = json_object_new_array(); root_for_each_workspace(ipc_get_workspaces_callback, workspaces); const char *json_string = json_object_to_json_string(workspaces); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(workspaces); // free goto exit_cleanup; } @@ -679,7 +690,7 @@ void ipc_client_handle_command(struct ipc_client *client) { struct json_object *request = json_tokener_parse(buf); if (request == NULL || !json_object_is_type(request, json_type_array)) { const char msg[] = "{\"success\": false}"; - client_valid = ipc_send_reply(client, msg, strlen(msg)); + ipc_send_reply(client, payload_type, msg, strlen(msg)); sway_log(SWAY_INFO, "Failed to parse subscribe request"); goto exit_cleanup; } @@ -707,7 +718,7 @@ void ipc_client_handle_command(struct ipc_client *client) { is_tick = true; } else { const char msg[] = "{\"success\": false}"; - client_valid = ipc_send_reply(client, msg, strlen(msg)); + ipc_send_reply(client, payload_type, msg, strlen(msg)); json_object_put(request); sway_log(SWAY_INFO, "Unsupported event type in subscribe request"); goto exit_cleanup; @@ -716,11 +727,11 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object_put(request); const char msg[] = "{\"success\": true}"; - client_valid = ipc_send_reply(client, msg, strlen(msg)); + ipc_send_reply(client, payload_type, msg, strlen(msg)); if (is_tick) { - client->current_command = IPC_EVENT_TICK; const char tickmsg[] = "{\"first\": true, \"payload\": \"\"}"; - ipc_send_reply(client, tickmsg, strlen(tickmsg)); + ipc_send_reply(client, IPC_EVENT_TICK, tickmsg, + strlen(tickmsg)); } goto exit_cleanup; } @@ -733,8 +744,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object_array_add(inputs, ipc_json_describe_input(device)); } const char *json_string = json_object_to_json_string(inputs); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(inputs); // free goto exit_cleanup; } @@ -747,8 +758,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object_array_add(seats, ipc_json_describe_seat(seat)); } const char *json_string = json_object_to_json_string(seats); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(seats); // free goto exit_cleanup; } @@ -757,8 +768,8 @@ void ipc_client_handle_command(struct ipc_client *client) { { json_object *tree = ipc_json_describe_node_recursive(&root->node); const char *json_string = json_object_to_json_string(tree); - client_valid = - ipc_send_reply(client, json_string, (uint32_t) strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(tree); goto exit_cleanup; } @@ -768,8 +779,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object *marks = json_object_new_array(); root_for_each_container(ipc_get_marks_callback, marks); const char *json_string = json_object_to_json_string(marks); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(marks); goto exit_cleanup; } @@ -778,8 +789,8 @@ void ipc_client_handle_command(struct ipc_client *client) { { json_object *version = ipc_json_get_version(); const char *json_string = json_object_to_json_string(version); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(version); // free goto exit_cleanup; } @@ -794,9 +805,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object_array_add(bars, json_object_new_string(bar->id)); } const char *json_string = json_object_to_json_string(bars); - client_valid = - ipc_send_reply(client, json_string, - (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(bars); // free } else { // Send particular bar's details @@ -810,15 +820,14 @@ void ipc_client_handle_command(struct ipc_client *client) { } if (!bar) { const char *error = "{ \"success\": false, \"error\": \"No bar with that ID\" }"; - client_valid = - ipc_send_reply(client, error, (uint32_t)strlen(error)); + ipc_send_reply(client, payload_type, error, + (uint32_t)strlen(error)); goto exit_cleanup; } json_object *json = ipc_json_describe_bar_config(bar); const char *json_string = json_object_to_json_string(json); - client_valid = - ipc_send_reply(client, json_string, - (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(json); // free } goto exit_cleanup; @@ -832,8 +841,8 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object_array_add(modes, json_object_new_string(mode->name)); } const char *json_string = json_object_to_json_string(modes); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(modes); // free goto exit_cleanup; } @@ -843,34 +852,32 @@ void ipc_client_handle_command(struct ipc_client *client) { json_object *json = json_object_new_object(); json_object_object_add(json, "config", json_object_new_string(config->current_config)); const char *json_string = json_object_to_json_string(json); - client_valid = - ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); + ipc_send_reply(client, payload_type, json_string, + (uint32_t)strlen(json_string)); json_object_put(json); // free goto exit_cleanup; - } + } case IPC_SYNC: { // It was decided sway will not support this, just return success:false const char msg[] = "{\"success\": false}"; - ipc_send_reply(client, msg, strlen(msg)); + ipc_send_reply(client, payload_type, msg, strlen(msg)); goto exit_cleanup; } default: - sway_log(SWAY_INFO, "Unknown IPC command type %i", client->current_command); + sway_log(SWAY_INFO, "Unknown IPC command type %x", payload_type); goto exit_cleanup; } exit_cleanup: - if (client_valid) { - client->payload_length = 0; - } free(buf); return; } -bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t payload_length) { +bool ipc_send_reply(struct ipc_client *client, enum ipc_command_type payload_type, + const char *payload, uint32_t payload_length) { assert(payload); char data[IPC_HEADER_SIZE]; @@ -878,7 +885,7 @@ bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t pay memcpy(data, ipc_magic, sizeof(ipc_magic)); memcpy(&data32[0], &payload_length, sizeof(payload_length)); - memcpy(&data32[1], &client->current_command, sizeof(client->current_command)); + memcpy(&data32[1], &payload_type, sizeof(payload_type)); while (client->write_buffer_len + IPC_HEADER_SIZE + payload_length >= client->write_buffer_size) { @@ -910,6 +917,7 @@ bool ipc_send_reply(struct ipc_client *client, const char *payload, uint32_t pay ipc_client_handle_writable, client); } - sway_log(SWAY_DEBUG, "Added IPC reply to client %d queue: %s", client->fd, payload); + sway_log(SWAY_DEBUG, "Added IPC reply of type 0x%x to client %d queue: %s", + payload_type, client->fd, payload); return true; } -- cgit v1.2.3-54-g00ecf