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.
This commit is contained in:
Ashkan Kiani 2019-04-16 17:35:39 -07:00 committed by Brian Ashworth
parent 9ac2342a67
commit aa4deef8a8

View file

@ -47,13 +47,14 @@ struct ipc_client {
struct wl_event_source *writable_event_source; struct wl_event_source *writable_event_source;
struct sway_server *server; struct sway_server *server;
int fd; int fd;
uint32_t payload_length;
uint32_t security_policy; uint32_t security_policy;
enum ipc_command_type current_command;
enum ipc_command_type subscribed_events; enum ipc_command_type subscribed_events;
size_t write_buffer_len; size_t write_buffer_len;
size_t write_buffer_size; size_t write_buffer_size;
char *write_buffer; 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); 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_readable(int client_fd, uint32_t mask, void *data);
int ipc_client_handle_writable(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_disconnect(struct ipc_client *client);
void ipc_client_handle_command(struct ipc_client *client); void ipc_client_handle_command(struct ipc_client *client, uint32_t payload_length,
bool ipc_send_reply(struct ipc_client *client, const char *payload, 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) { static void handle_display_destroy(struct wl_listener *listener, void *data) {
if (ipc_event_source) { if (ipc_event_source) {
@ -178,7 +181,7 @@ int ipc_handle_connection(int fd, uint32_t mask, void *data) {
return 0; return 0;
} }
client->server = server; client->server = server;
client->payload_length = 0; client->pending_length = 0;
client->fd = client_fd; client->fd = client_fd;
client->subscribed_events = 0; client->subscribed_events = 0;
client->event_source = wl_event_loop_add_fd(server->wl_event_loop, 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 // Wait for the rest of the command payload in case the header has already been read
if (client->payload_length > 0) { if (client->pending_length > 0) {
if ((uint32_t)read_available >= client->payload_length) { if ((uint32_t)read_available >= client->pending_length) {
ipc_client_handle_command(client); // 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; return 0;
} }
@ -251,11 +258,15 @@ int ipc_client_handle_readable(int client_fd, uint32_t mask, void *data) {
return 0; return 0;
} }
memcpy(&client->payload_length, &buf32[0], sizeof(buf32[0])); memcpy(&client->pending_length, &buf32[0], sizeof(buf32[0]));
memcpy(&client->current_command, &buf32[1], sizeof(buf32[1])); memcpy(&client->pending_type, &buf32[1], sizeof(buf32[1]));
if (read_available - received >= (long)client->payload_length) { if (read_available - received >= (long)client->pending_length) {
ipc_client_handle_command(client); // 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; 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) { if ((client->subscribed_events & event_mask(event)) == 0) {
continue; continue;
} }
client->current_command = event; if (!ipc_send_reply(client, event, json_string,
if (!ipc_send_reply(client, json_string, (uint32_t) strlen(json_string))) { (uint32_t)strlen(json_string))) {
sway_log_errno(SWAY_INFO, "Unable to send reply to IPC client"); sway_log_errno(SWAY_INFO, "Unable to send reply to IPC client");
/* ipc_send_reply destroys client on error, which also /* ipc_send_reply destroys client on error, which also
* removes it from the list, so we need to process * 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")) { if (!sway_assert(client != NULL, "client != NULL")) {
return; return;
} }
char *buf = malloc(client->payload_length + 1); char *buf = malloc(payload_length + 1);
if (!buf) { if (!buf) {
sway_log_errno(SWAY_INFO, "Unable to allocate IPC payload"); sway_log_errno(SWAY_INFO, "Unable to allocate IPC payload");
ipc_client_disconnect(client); ipc_client_disconnect(client);
return; return;
} }
if (client->payload_length > 0) { if (payload_length > 0) {
// Payload should be fully available // 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) if (received == -1)
{ {
sway_log_errno(SWAY_INFO, "Unable to receive payload from IPC client"); 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; return;
} }
} }
buf[client->payload_length] = '\0'; buf[payload_length] = '\0';
bool client_valid = true; switch (payload_type) {
switch (client->current_command) {
case IPC_COMMAND: case IPC_COMMAND:
{ {
char *line = strtok(buf, "\n"); char *line = strtok(buf, "\n");
while (line) { while (line) {
size_t line_length = strlen(line); size_t line_length = strlen(line);
if (line + line_length >= buf + client->payload_length) { if (line + line_length >= buf + payload_length) {
break; break;
} }
line[line_length] = ';'; line[line_length] = ';';
@ -609,7 +620,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
transaction_commit_dirty(); transaction_commit_dirty();
char *json = cmd_results_to_json(res_list); char *json = cmd_results_to_json(res_list);
int length = strlen(json); 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); free(json);
while (res_list->length) { while (res_list->length) {
struct cmd_results *results = res_list->items[0]; 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: case IPC_SEND_TICK:
{ {
ipc_event_tick(buf); ipc_event_tick(buf);
ipc_send_reply(client, "{\"success\": true}", 17); ipc_send_reply(client, payload_type, "{\"success\": true}", 17);
goto exit_cleanup; 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); const char *json_string = json_object_to_json_string(outputs);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(outputs); // free json_object_put(outputs); // free
goto exit_cleanup; goto exit_cleanup;
} }
@ -667,8 +678,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
json_object *workspaces = json_object_new_array(); json_object *workspaces = json_object_new_array();
root_for_each_workspace(ipc_get_workspaces_callback, workspaces); root_for_each_workspace(ipc_get_workspaces_callback, workspaces);
const char *json_string = json_object_to_json_string(workspaces); const char *json_string = json_object_to_json_string(workspaces);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(workspaces); // free json_object_put(workspaces); // free
goto exit_cleanup; goto exit_cleanup;
} }
@ -679,7 +690,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
struct json_object *request = json_tokener_parse(buf); struct json_object *request = json_tokener_parse(buf);
if (request == NULL || !json_object_is_type(request, json_type_array)) { if (request == NULL || !json_object_is_type(request, json_type_array)) {
const char msg[] = "{\"success\": false}"; 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"); sway_log(SWAY_INFO, "Failed to parse subscribe request");
goto exit_cleanup; goto exit_cleanup;
} }
@ -707,7 +718,7 @@ void ipc_client_handle_command(struct ipc_client *client) {
is_tick = true; is_tick = true;
} else { } else {
const char msg[] = "{\"success\": false}"; 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); json_object_put(request);
sway_log(SWAY_INFO, "Unsupported event type in subscribe request"); sway_log(SWAY_INFO, "Unsupported event type in subscribe request");
goto exit_cleanup; goto exit_cleanup;
@ -716,11 +727,11 @@ void ipc_client_handle_command(struct ipc_client *client) {
json_object_put(request); json_object_put(request);
const char msg[] = "{\"success\": true}"; 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) { if (is_tick) {
client->current_command = IPC_EVENT_TICK;
const char tickmsg[] = "{\"first\": true, \"payload\": \"\"}"; 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; 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)); json_object_array_add(inputs, ipc_json_describe_input(device));
} }
const char *json_string = json_object_to_json_string(inputs); const char *json_string = json_object_to_json_string(inputs);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(inputs); // free json_object_put(inputs); // free
goto exit_cleanup; 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)); json_object_array_add(seats, ipc_json_describe_seat(seat));
} }
const char *json_string = json_object_to_json_string(seats); const char *json_string = json_object_to_json_string(seats);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(seats); // free json_object_put(seats); // free
goto exit_cleanup; 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); json_object *tree = ipc_json_describe_node_recursive(&root->node);
const char *json_string = json_object_to_json_string(tree); const char *json_string = json_object_to_json_string(tree);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t) strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(tree); json_object_put(tree);
goto exit_cleanup; goto exit_cleanup;
} }
@ -768,8 +779,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
json_object *marks = json_object_new_array(); json_object *marks = json_object_new_array();
root_for_each_container(ipc_get_marks_callback, marks); root_for_each_container(ipc_get_marks_callback, marks);
const char *json_string = json_object_to_json_string(marks); const char *json_string = json_object_to_json_string(marks);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(marks); json_object_put(marks);
goto exit_cleanup; goto exit_cleanup;
} }
@ -778,8 +789,8 @@ void ipc_client_handle_command(struct ipc_client *client) {
{ {
json_object *version = ipc_json_get_version(); json_object *version = ipc_json_get_version();
const char *json_string = json_object_to_json_string(version); const char *json_string = json_object_to_json_string(version);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(version); // free json_object_put(version); // free
goto exit_cleanup; 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)); json_object_array_add(bars, json_object_new_string(bar->id));
} }
const char *json_string = json_object_to_json_string(bars); const char *json_string = json_object_to_json_string(bars);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
(uint32_t)strlen(json_string));
json_object_put(bars); // free json_object_put(bars); // free
} else { } else {
// Send particular bar's details // Send particular bar's details
@ -810,15 +820,14 @@ void ipc_client_handle_command(struct ipc_client *client) {
} }
if (!bar) { if (!bar) {
const char *error = "{ \"success\": false, \"error\": \"No bar with that ID\" }"; const char *error = "{ \"success\": false, \"error\": \"No bar with that ID\" }";
client_valid = ipc_send_reply(client, payload_type, error,
ipc_send_reply(client, error, (uint32_t)strlen(error)); (uint32_t)strlen(error));
goto exit_cleanup; goto exit_cleanup;
} }
json_object *json = ipc_json_describe_bar_config(bar); json_object *json = ipc_json_describe_bar_config(bar);
const char *json_string = json_object_to_json_string(json); const char *json_string = json_object_to_json_string(json);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string));
(uint32_t)strlen(json_string));
json_object_put(json); // free json_object_put(json); // free
} }
goto exit_cleanup; 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)); json_object_array_add(modes, json_object_new_string(mode->name));
} }
const char *json_string = json_object_to_json_string(modes); const char *json_string = json_object_to_json_string(modes);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(modes); // free json_object_put(modes); // free
goto exit_cleanup; 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 *json = json_object_new_object();
json_object_object_add(json, "config", json_object_new_string(config->current_config)); json_object_object_add(json, "config", json_object_new_string(config->current_config));
const char *json_string = json_object_to_json_string(json); const char *json_string = json_object_to_json_string(json);
client_valid = ipc_send_reply(client, payload_type, json_string,
ipc_send_reply(client, json_string, (uint32_t)strlen(json_string)); (uint32_t)strlen(json_string));
json_object_put(json); // free json_object_put(json); // free
goto exit_cleanup; goto exit_cleanup;
} }
case IPC_SYNC: case IPC_SYNC:
{ {
// It was decided sway will not support this, just return success:false // It was decided sway will not support this, just return success:false
const char msg[] = "{\"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; goto exit_cleanup;
} }
default: 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; goto exit_cleanup;
} }
exit_cleanup: exit_cleanup:
if (client_valid) {
client->payload_length = 0;
}
free(buf); free(buf);
return; 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); assert(payload);
char data[IPC_HEADER_SIZE]; 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(data, ipc_magic, sizeof(ipc_magic));
memcpy(&data32[0], &payload_length, sizeof(payload_length)); 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 >= while (client->write_buffer_len + IPC_HEADER_SIZE + payload_length >=
client->write_buffer_size) { 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); 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; return true;
} }