<div dir="ltr"> <div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 17, 2014 at 11:20 AM, Ander Conselvan de Oliveira <<a href="mailto:conselvan2@gmail.com" target="_blank">conselvan2@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Ander Conselvan de Oliveira <<a href="mailto:ander.conselvan.de.oliveira@intel.com">ander.conselvan.de.oliveira@intel.com</a>> If a message was too big to fit in the connection buffer, the code in wl_buffer_put would just write past the end of it. I haven't seen any real world use case that would trigger this bug, but it was possible to trigger it by sending a long enough string to the wl_data_source.offer request. </blockquote><div> </div><div>This will flat out kill the client, no? It seems like we should be able to handle that more gracefully; does the client have any idea what the buffer size is? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <a href="https://bugs.freedesktop.org/show_bug.cgi?id=69267" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=69267</a> --- src/connection.c | 25 +++++++++++++++++-------- tests/connection-test.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/connection.c b/src/connection.c index 40a2182..63b0592 100644 --- a/src/connection.c +++ b/src/connection.c @@ -61,11 +61,18 @@ struct wl_connection { int want_flush; }; -static void +static int wl_buffer_put(struct wl_buffer *b, const void *data, size_t count) { uint32_t head, size; + if (count > sizeof(b->data)) { + wl_log("Data too big for buffer (%d > %d).\n", + count, sizeof(b->data)); + errno = E2BIG; + return -1; + } + head = MASK(b->head); if (head + count <= sizeof b->data) { memcpy(b->data + head, data, count); @@ -76,6 +83,8 @@ wl_buffer_put(struct wl_buffer *b, const void *data, size_t count) } b->head += count; + + return 0; } static void @@ -243,8 +252,8 @@ decode_cmsg(struct wl_buffer *buffer, struct msghdr *msg) size /= sizeof(int32_t); for (i = 0; i < size; i++) close(((int*)CMSG_DATA(cmsg))[i]); - } else { - wl_buffer_put(buffer, CMSG_DATA(cmsg), size); + } else if (wl_buffer_put(buffer, CMSG_DATA(cmsg), size) < 0) { + return -1; } } @@ -350,7 +359,9 @@ wl_connection_write(struct wl_connection *connection, return -1; } - wl_buffer_put(&connection->out, data, count); + if (wl_buffer_put(&connection->out, data, count) < 0) + return -1; + connection->want_flush = 1; return 0; @@ -367,7 +378,7 @@ wl_connection_queue(struct wl_connection *connection, return -1; } - wl_buffer_put(&connection->out, data, count); + return wl_buffer_put(&connection->out, data, count); return 0; </blockquote><div> </div><div>You forgot to remove the "return 0;" here. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> } @@ -394,9 +405,7 @@ wl_connection_put_fd(struct wl_connection *connection, int32_t fd) return -1; } - wl_buffer_put(&connection->fds_out, &fd, sizeof fd); - - return 0; + return wl_buffer_put(&connection->fds_out, &fd, sizeof fd); } const char * diff --git a/tests/connection-test.c b/tests/connection-test.c index 52d235d..1213875 100644 --- a/tests/connection-test.c +++ b/tests/connection-test.c @@ -235,6 +235,27 @@ expected_fail_marshal(int expected_error, const char *format, ...) assert(errno == expected_error); } +static void +expected_fail_marshal_send(struct marshal_data *data, int expected_error, + const char *format, ...) +{ + struct wl_closure *closure; + static const uint32_t opcode = 4444; + static struct wl_object sender = { NULL, NULL, 1234 }; + struct wl_message message = { "test", format, NULL }; + va_list ap; + + va_start(ap, format); + closure = wl_closure_vmarshal(&sender, opcode, ap, &message); + va_end(ap); + + assert(closure); + assert(wl_closure_send(closure, data->write_connection) < 0); + assert(errno == expected_error); + + wl_closure_destroy(closure); +} + TEST(connection_marshal_nullables) { struct marshal_data data; @@ -490,6 +511,22 @@ TEST(connection_marshal_alot) release_marshal_data(&data); } +TEST(connection_marshal_too_big) +{ + struct marshal_data data; + char *big_string = malloc(5000); + + memset(big_string, ' ', 4999); + big_string[4999] = '\0'; + + setup_marshal_data(&data); + + expected_fail_marshal_send(&data, E2BIG, "s", big_string); + + release_marshal_data(&data); + free(big_string); +} + static void marshal_helper(const char *format, void *handler, ...) { -- 1.8.3.2 _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote></div> -- Jasper </div></div>