]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
whitelist: Use a watcher for control socket reading rather than blocking
authorRob Shearman <rob@graphiant.com>
Wed, 1 Jun 2022 18:41:08 +0000 (19:41 +0100)
committerTobias Brunner <tobias@strongswan.org>
Tue, 15 Jul 2025 12:50:56 +0000 (14:50 +0200)
Performing a stream read_all call (which is a blocking read) from
within the accept callback has the issue that if a whitelist client is
still connected whilst a shutdown of the charon deamon is triggered
then that shutdown won't complete gracefully due to the accept task
never exiting.

So fix shutting down gracefully by using the socket watcher rather than
a blocking read upon connection accept. Fall back to a blocking read
for partial messages to avoid the complexity associated (i.e. storing
state) for incomplete reads, which shouldn't block and cause the
original problem if the client only sends whole messages.

src/libcharon/plugins/whitelist/whitelist_control.c

index e3a787ca70bad695a4a3c93cfcba1f4b0d24d1c5..c8dc7ef8d06c9b321cf978d57102256a7eebcaf6 100644 (file)
@@ -95,13 +95,32 @@ static void list(private_whitelist_control_t *this,
 /**
  * Dispatch a received message
  */
-static bool on_accept(private_whitelist_control_t *this, stream_t *stream)
+CALLBACK(on_read, bool,
+       private_whitelist_control_t *this, stream_t *stream)
 {
        identification_t *id;
        whitelist_msg_t msg;
+       ssize_t n;
 
-       while (stream->read_all(stream, &msg, sizeof(msg)))
+       while (TRUE)
        {
+               n = stream->read(stream, &msg, sizeof(msg), FALSE);
+               if (n <= 0)
+               {
+                       if (errno == EWOULDBLOCK)
+                       {
+                               break;
+                       }
+                       return FALSE;
+               }
+               if (n < sizeof(msg))
+               {
+                       if (!stream->read_all(stream, ((char *)&msg) + n, sizeof(msg) - n))
+                       {
+                               return FALSE;
+                       }
+               }
+
                msg.id[sizeof(msg.id) - 1] = 0;
                id = identification_create_from_string(msg.id);
                switch (ntohl(msg.type))
@@ -131,7 +150,14 @@ static bool on_accept(private_whitelist_control_t *this, stream_t *stream)
                id->destroy(id);
        }
 
-       return FALSE;
+       return TRUE;
+}
+
+CALLBACK(on_accept, bool,
+       private_whitelist_control_t *this, stream_t *stream)
+{
+       stream->on_read(stream, on_read, this);
+       return TRUE;
 }
 
 METHOD(whitelist_control_t, destroy, void,