From 104d83d5f5fd32ab29219a54527071394441ca27 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 2 Sep 2020 13:34:51 -0400 Subject: [PATCH 1/2] boot-server: Ref count the connections This commit adds reference counting to ply_boot_connection_t. This will be needed by a subsequent commit to fix a crasher bug. --- src/ply-boot-server.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/ply-boot-server.c b/src/ply-boot-server.c index 55195506..0e85e748 100644 --- a/src/ply-boot-server.c +++ b/src/ply-boot-server.c @@ -47,6 +47,8 @@ typedef struct uid_t uid; pid_t pid; + int reference_count; + uint32_t credentials_read : 1; } ply_boot_connection_t; @@ -165,6 +167,7 @@ ply_boot_connection_new (ply_boot_server_t *server, connection->fd = fd; connection->server = server; connection->watch = NULL; + connection->reference_count = 1; return connection; } @@ -179,6 +182,26 @@ ply_boot_connection_free (ply_boot_connection_t *connection) free (connection); } +static void +ply_boot_connection_take_reference (ply_boot_connection_t *connection) +{ + connection->reference_count++; +} + +static void +ply_boot_connection_drop_reference (ply_boot_connection_t *connection) +{ + if (connection == NULL) + return; + + connection->reference_count--; + + assert (connection->reference_count >= 0); + + if (connection->reference_count == 0) + ply_boot_connection_free (connection); +} + bool ply_boot_server_listen (ply_boot_server_t *server) { @@ -713,7 +736,7 @@ ply_boot_connection_on_hangup (ply_boot_connection_t *connection) assert (node != NULL); - ply_boot_connection_free (connection); + ply_boot_connection_drop_reference (connection); ply_list_remove_node (server->connections, node); } From 9f892393675d19d17b900fd12c9c5cb27762a8e3 Mon Sep 17 00:00:00 2001 From: Ray Strode Date: Wed, 2 Sep 2020 13:44:37 -0400 Subject: [PATCH 2/2] boot-server: Handle client disconnecting while trigger pending At the moment if a client disconnects while the daemon is completely an asynchronous request, the daemon crashes trying to access a freed connection object. This commit changes the boot server code to keep the connection object alive after the client disconnects, if there's pending work to do. https://gitlab.freedesktop.org/plymouth/plymouth/-/issues/125 --- src/ply-boot-server.c | 49 +++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/ply-boot-server.c b/src/ply-boot-server.c index 0e85e748..173063f7 100644 --- a/src/ply-boot-server.c +++ b/src/ply-boot-server.c @@ -50,6 +50,7 @@ typedef struct int reference_count; uint32_t credentials_read : 1; + uint32_t disconnected : 1; } ply_boot_connection_t; struct _ply_boot_server @@ -315,30 +316,43 @@ ply_boot_connection_on_password_answer (ply_boot_connection_t *connection, { ply_trace ("got password answer"); - ply_boot_connection_send_answer (connection, password); + if (!connection->disconnected) + ply_boot_connection_send_answer (connection, password); + if (password != NULL) ply_list_append_data (connection->server->cached_passwords, strdup (password)); + + ply_boot_connection_drop_reference (connection); } static void ply_boot_connection_on_deactivated (ply_boot_connection_t *connection) { ply_trace ("deactivated"); - if (!ply_write (connection->fd, - PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK, - strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK))) - ply_trace ("could not finish writing deactivate reply: %m"); + + if (!connection->disconnected) { + if (!ply_write (connection->fd, + PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK, + strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK))) + ply_trace ("could not finish writing deactivate reply: %m"); + } + + ply_boot_connection_drop_reference (connection); } static void ply_boot_connection_on_quit_complete (ply_boot_connection_t *connection) { ply_trace ("quit complete"); - if (!ply_write (connection->fd, - PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK, - strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK))) - ply_trace ("could not finish writing quit reply: %m"); + if (!connection->disconnected) { + if (!ply_write (connection->fd, + PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK, + strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ACK))) + ply_trace ("could not finish writing quit reply: %m"); + } + + ply_boot_connection_drop_reference (connection); } static void @@ -346,7 +360,10 @@ ply_boot_connection_on_question_answer (ply_boot_connection_t *connection, const char *answer) { ply_trace ("got question answer: %s", answer); - ply_boot_connection_send_answer (connection, answer); + if (!connection->disconnected) + ply_boot_connection_send_answer (connection, answer); + + ply_boot_connection_drop_reference (connection); } static void @@ -354,7 +371,10 @@ ply_boot_connection_on_keystroke_answer (ply_boot_connection_t *connection, const char *key) { ply_trace ("got key: %s", key); - ply_boot_connection_send_answer (connection, key); + if (!connection->disconnected) + ply_boot_connection_send_answer (connection, key); + + ply_boot_connection_drop_reference (connection); } static void @@ -487,6 +507,7 @@ ply_boot_connection_on_request (ply_boot_connection_t *connection) (ply_trigger_handler_t) ply_boot_connection_on_deactivated, connection); + ply_boot_connection_take_reference (connection); if (server->deactivate_handler != NULL) server->deactivate_handler (server->user_data, deactivate_trigger, server); @@ -514,6 +535,7 @@ ply_boot_connection_on_request (ply_boot_connection_t *connection) (ply_trigger_handler_t) ply_boot_connection_on_quit_complete, connection); + ply_boot_connection_take_reference (connection); if (server->quit_handler != NULL) server->quit_handler (server->user_data, retain_splash, quit_trigger, server); @@ -533,6 +555,7 @@ ply_boot_connection_on_request (ply_boot_connection_t *connection) (ply_trigger_handler_t) ply_boot_connection_on_password_answer, connection); + ply_boot_connection_take_reference (connection); if (server->ask_for_password_handler != NULL) { server->ask_for_password_handler (server->user_data, @@ -617,6 +640,7 @@ ply_boot_connection_on_request (ply_boot_connection_t *connection) (ply_trigger_handler_t) ply_boot_connection_on_question_answer, connection); + ply_boot_connection_take_reference (connection); if (server->ask_question_handler != NULL) { server->ask_question_handler (server->user_data, @@ -649,6 +673,7 @@ ply_boot_connection_on_request (ply_boot_connection_t *connection) (ply_trigger_handler_t) ply_boot_connection_on_keystroke_answer, connection); + ply_boot_connection_take_reference (connection); if (server->watch_for_keystroke_handler != NULL) { server->watch_for_keystroke_handler (server->user_data, @@ -730,6 +755,8 @@ ply_boot_connection_on_hangup (ply_boot_connection_t *connection) assert (connection != NULL); assert (connection->server != NULL); + connection->disconnected = true; + server = connection->server; node = ply_list_find_node (server->connections, connection);