diff --git a/src/connection.c b/src/connection.c index 2d1e8d1d..043ddfb8 100644 --- a/src/connection.c +++ b/src/connection.c @@ -93,19 +93,45 @@ ring_buffer_mask(const struct wl_ring_buffer *b, size_t i) { return i & m; } +static size_t +ring_buffer_size(struct wl_ring_buffer *b) +{ + return b->head - b->tail; +} + +/* Precondition: the data will not overflow the buffer */ static int ring_buffer_put(struct wl_ring_buffer *b, const void *data, size_t count) { - size_t head, size; + size_t head, size, buffer_size, capacity; + + if (b->head < b->tail) { + wl_abort("ring_buffer_put: ring buffer corrupt, %zu < %zu\n", + b->head, b->tail); + } + + capacity = ring_buffer_capacity(b); + buffer_size = ring_buffer_size(b); + if (buffer_size > capacity) { + wl_abort("ring_buffer_put: ring buffer corrupt: " + "%zu - %zu > %zu\n", b->head, b->tail, capacity); + } if (count == 0) return 0; + if (capacity - buffer_size < count) { + wl_abort("ring_buffer_put: attempt to overfill buffer: " + "%zu - %zu < %zu\n", capacity, buffer_size, count); + } + head = ring_buffer_mask(b, b->head); - if (head + count <= ring_buffer_capacity(b)) { + size = capacity - head; + if (count <= size) { + /* Enough space after head to fulfill request */ memcpy(b->data + head, data, count); } else { - size = ring_buffer_capacity(b) - head; + /* Need to wrap around */ memcpy(b->data + head, data, size); memcpy(b->data, (const char *) data + size, count - size); } @@ -115,78 +141,160 @@ ring_buffer_put(struct wl_ring_buffer *b, const void *data, size_t count) return 0; } +/* Precondition: the buffer is not full */ static void ring_buffer_put_iov(struct wl_ring_buffer *b, struct iovec *iov, int *count) { - size_t head, tail; + size_t head, tail, size, capacity; + + if (b->head < b->tail) { + wl_abort("ring_buffer_put_iov: ring buffer corrupt, %zu < %zu\n", + b->head, b->tail); + } + + size = ring_buffer_size(b); + capacity = ring_buffer_capacity(b); + if (size >= capacity) { + wl_abort("ring_buffer_put_iov: ring buffer full or corrupt: " + "%zu - %zu >= %zu\n", b->head, b->tail, capacity); + } head = ring_buffer_mask(b, b->head); tail = ring_buffer_mask(b, b->tail); if (head < tail) { + /* Buffer is like this: + * head tail + * | | + * +---------+-----------------+---------+ + * | VALID | INVALID | VALID | + * +---------+-----------------+---------+ + */ iov[0].iov_base = b->data + head; iov[0].iov_len = tail - head; *count = 1; } else if (tail == 0) { + /* Buffer is like this: + * tail head + * | | + * +---------------------------+---------+ + * | VALID | INVALID | + * +---------------------------+---------+ + */ iov[0].iov_base = b->data + head; - iov[0].iov_len = ring_buffer_capacity(b) - head; + iov[0].iov_len = capacity - head; *count = 1; } else { + /* Buffer is like this: + * tail head + * | | + * +---------------------------+---------+ + * | INVALID | VALID | INVALID | + * +---------------------------+---------+ + */ iov[0].iov_base = b->data + head; - iov[0].iov_len = ring_buffer_capacity(b) - head; + iov[0].iov_len = capacity - head; iov[1].iov_base = b->data; iov[1].iov_len = tail; *count = 2; } } +/* Precondition: the buffer is not empty */ static void ring_buffer_get_iov(struct wl_ring_buffer *b, struct iovec *iov, int *count) { - size_t head, tail; + size_t head, tail, capacity; + + if (b->head <= b->tail) { + wl_abort("ring_buffer_get_iov(): empty or corrupt buffer: %zu <= %zu\n", + b->head, b->tail); + } + + capacity = ring_buffer_capacity(b); + if (ring_buffer_size(b) > capacity) { + wl_abort("ring_buffer_put_iov: ring buffer corrupt: " + "%zu - %zu > %zu\n", b->head, b->tail, capacity); + } head = ring_buffer_mask(b, b->head); tail = ring_buffer_mask(b, b->tail); if (tail < head) { + /* Buffer is like this: + * tail head + * | | + * +---------+-----------------+---------+ + * | INVALID | VALID | INVALID | + * +---------+-----------------+---------+ + */ iov[0].iov_base = b->data + tail; iov[0].iov_len = head - tail; *count = 1; } else if (head == 0) { + /* Buffer is like this: + * head tail + * | | + * +---------------------------+---------+ + * | INVALID | VALID | + * +---------------------------+---------+ + */ iov[0].iov_base = b->data + tail; - iov[0].iov_len = ring_buffer_capacity(b) - tail; + iov[0].iov_len = capacity - tail; *count = 1; } else { + /* Buffer is like this: + * head tail + * | | + * +-------+-------------------+---------+ + * | VALID | INVALID | VALID | + * +---------------------------+---------+ + */ iov[0].iov_base = b->data + tail; - iov[0].iov_len = ring_buffer_capacity(b) - tail; + iov[0].iov_len = capacity - tail; iov[1].iov_base = b->data; iov[1].iov_len = head; *count = 2; } } +/* Precondition: the data will not underflow the buffer */ static void ring_buffer_copy(struct wl_ring_buffer *b, void *data, size_t count) { - size_t tail, size; + size_t tail, size, buffer_size, capacity; + + if (b->head < b->tail) { + wl_abort("ring_buffer_copy(): ring buffer corrupt, %zu < %zu\n", + b->head, b->tail); + } + + buffer_size = ring_buffer_size(b); + capacity = ring_buffer_capacity(b); + if (buffer_size > capacity) { + wl_abort("ring_buffer_copy(): ring buffer corrupt: " + "%zu - %zu > %zu\n", b->head, b->tail, capacity); + } if (count == 0) return; + if (buffer_size < count) { + wl_abort("ring_buffer_copy(): attempt to copy %zu bytes " + "but buffer has %zu bytes\n", + count, buffer_size); + } + tail = ring_buffer_mask(b, b->tail); - if (tail + count <= ring_buffer_capacity(b)) { + size = capacity - tail; + if (count <= size) { + /* Enough data after the tail to fulfill the request */ memcpy(data, b->data + tail, count); } else { - size = ring_buffer_capacity(b) - tail; + /* Must wrap buffer around */ memcpy(data, b->data + tail, size); memcpy((char *) data + size, b->data, count - size); } } -static size_t -ring_buffer_size(struct wl_ring_buffer *b) -{ - return b->head - b->tail; -} - static char * ring_buffer_tail(const struct wl_ring_buffer *b) { @@ -415,7 +523,7 @@ decode_cmsg(struct wl_ring_buffer *buffer, struct msghdr *msg) { struct cmsghdr *cmsg; size_t size, i; - int overflow = 0; + bool overflow = false; for (cmsg = CMSG_FIRSTHDR(msg); cmsg != NULL; cmsg = CMSG_NXTHDR(msg, cmsg)) { @@ -426,7 +534,7 @@ decode_cmsg(struct wl_ring_buffer *buffer, struct msghdr *msg) size = cmsg->cmsg_len - CMSG_LEN(0); if (ring_buffer_ensure_space(buffer, size) < 0 || overflow) { - overflow = 1; + overflow = true; size /= sizeof(int32_t); for (i = 0; i < size; i++) close(((int*)CMSG_DATA(cmsg))[i]); @@ -458,6 +566,9 @@ wl_connection_flush(struct wl_connection *connection) tail = connection->out.tail; while (ring_buffer_size(&connection->out) > 0) { + /* Ring buffer is not empty, so this is safe. */ + ring_buffer_get_iov(&connection->out, iov, &count); + build_cmsg(&connection->fds_out, cmsg, &clen); if (clen >= CLEN) { @@ -532,6 +643,7 @@ wl_connection_read(struct wl_connection *connection) if (ring_buffer_ensure_space(&connection->in, 1) < 0) return -1; + /* Ring buffer is not full, so this is safe. */ ring_buffer_put_iov(&connection->in, iov, &count); msg.msg_name = NULL; @@ -685,6 +797,7 @@ wl_message_get_since(const struct wl_message *message) { int since; + /* This is trusted input */ since = atoi(message->signature); if (since == 0) @@ -952,14 +1065,14 @@ wl_connection_demarshal(struct wl_connection *connection, case WL_ARG_STRING: length = *p++; - if (length == 0 && !arg.nullable) { - wl_log("NULL string received on non-nullable " - "type, message %s(%s)\n", message->name, - message->signature); - errno = EINVAL; - goto err; - } if (length == 0) { + if (!arg.nullable) { + wl_log("NULL string received on non-nullable " + "type, message %s(%s)\n", message->name, + message->signature); + errno = EINVAL; + goto err; + } closure->args[i].s = NULL; break; } @@ -1062,7 +1175,10 @@ wl_connection_demarshal(struct wl_connection *connection, goto err; } + /* This ring buffer will always have a multiple of sizeof(int) + * bytes in it. */ ring_buffer_copy(&connection->fds_in, &fd, sizeof fd); + /* This can wrap but that is okay. */ connection->fds_in.tail += sizeof fd; closure->args[i].h = fd; break; diff --git a/src/wayland-server.c b/src/wayland-server.c index c81d98f1..cdcdb642 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -740,8 +740,7 @@ wl_client_post_implementation_error(struct wl_client *client, WL_EXPORT void wl_resource_post_no_memory(struct wl_resource *resource) { - wl_resource_post_error(resource->client->display_resource, - WL_DISPLAY_ERROR_NO_MEMORY, "no memory"); + wl_client_post_no_memory(resource->client); } /** Detect if a wl_resource uses the deprecated public definition.