From 8ae9cf4698b4fadae8cfbfbc801cf93d2385629d Mon Sep 17 00:00:00 2001
From: Bryan Jacobs <brjacobs@atlassian.com>
Date: Sun, 27 Mar 2022 13:07:29 +1100
Subject: [PATCH] Revert "libnm: buffer reads in
 nm_vpn_service_plugin_read_vpn_details()"

This partially reverts commit 4a9fcb0fc32e, which replaced one-byte
reads with buffered ones in the VPN service plugin.

Unfortunately the buffering means that commands coming after the magic
"DONE" string were being pulled into the buffer. Secrets agents expect
a "QUIT" to come after the "DONE", and since with buffering "QUIT" was
in the buffer, this led to a twenty-second delay on every VPN
connection using a secrets manager.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1164

Fixes: 4a9fcb0fc32e ('libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()')
---
 src/libnm-client-impl/nm-vpn-service-plugin.c | 84 +++++--------------
 1 file changed, 20 insertions(+), 64 deletions(-)

diff --git a/src/libnm-client-impl/nm-vpn-service-plugin.c b/src/libnm-client-impl/nm-vpn-service-plugin.c
index 2a217502de..7b04c30799 100644
--- a/src/libnm-client-impl/nm-vpn-service-plugin.c
+++ b/src/libnm-client-impl/nm-vpn-service-plugin.c
@@ -729,54 +729,6 @@ nm_vpn_service_plugin_secrets_required(NMVpnServicePlugin *plugin,
 
 /*****************************************************************************/
 
-typedef struct {
-    char *buf;
-    gsize n_buf;
-    int   fd;
-    bool  eof : 1;
-    char  buf_full[1024];
-} ReadFdBuf;
-
-static inline gboolean
-_read_fd_buf_c(ReadFdBuf *read_buf, char *ch)
-{
-    gssize n_read;
-
-    if (read_buf->n_buf > 0)
-        goto out_data;
-    if (read_buf->eof)
-        return FALSE;
-
-again:
-    n_read = read(read_buf->fd, read_buf->buf_full, sizeof(read_buf->buf_full));
-    if (n_read <= 0) {
-        if (n_read < 0 && errno == EAGAIN) {
-            struct pollfd pfd;
-            int           r;
-
-            memset(&pfd, 0, sizeof(pfd));
-            pfd.fd     = read_buf->fd;
-            pfd.events = POLLIN;
-
-            r = poll(&pfd, 1, -1);
-            if (r > 0)
-                goto again;
-            /* error or timeout. Fall through and set EOF. */
-        }
-        read_buf->eof = TRUE;
-        return FALSE;
-    }
-
-    read_buf->buf   = read_buf->buf_full;
-    read_buf->n_buf = n_read;
-
-out_data:
-    read_buf->n_buf--;
-    *ch = read_buf->buf[0];
-    read_buf->buf++;
-    return TRUE;
-}
-
 #define DATA_KEY_TAG   "DATA_KEY="
 #define DATA_VAL_TAG   "DATA_VAL="
 #define SECRET_KEY_TAG "SECRET_KEY="
@@ -808,7 +760,7 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
     nm_auto_free_gstring GString  *val     = NULL;
     nm_auto_free_gstring GString  *line    = NULL;
     GString                       *str     = NULL;
-    ReadFdBuf                      read_buf;
+    char                           c;
 
     if (out_data)
         g_return_val_if_fail(*out_data == NULL, FALSE);
@@ -819,27 +771,31 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
     secrets =
         g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, (GDestroyNotify) nm_free_secret);
 
-    read_buf.buf   = NULL;
-    read_buf.n_buf = 0;
-    read_buf.fd    = fd;
-    read_buf.eof   = FALSE;
-
     line = g_string_new(NULL);
 
     /* Read stdin for data and secret items until we get a DONE */
     while (1) {
-        gboolean eof;
-        char     c = '\0';
+        ssize_t nr;
 
-        eof = !_read_fd_buf_c(&read_buf, &c);
+        nr = read(fd, &c, 1);
+        if (nr < 0) {
+            if (errno == EAGAIN) {
+                struct pollfd pfd;
+                int           r;
 
-        if (!eof && c == '\0') {
-            /* On the first '\0' char, we also assume the data is finished. Abort. */
-            read_buf.eof = TRUE;
-            eof          = TRUE;
+                memset(&pfd, 0, sizeof(pfd));
+                pfd.fd     = fd;
+                pfd.events = POLLIN;
+
+                r = poll(&pfd, 1, -1);
+                if (r > 0)
+                    continue;
+
+                /* error or timeout. Fall through and break. */
+            }
+            break;
         }
-
-        if (!eof && c != '\n') {
+        if (nr > 0 && c != '\n') {
             g_string_append_c(line, c);
             if (line->len > 512 * 1024) {
                 /* we are about to read a huge line. That is not right, abort. */
@@ -893,7 +849,7 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
 next:
         g_string_truncate(line, 0);
 
-        if (eof)
+        if (nr == 0)
             break;
     }