Revert "libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()"
This partially reverts commit4a9fcb0fc3
, 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:4a9fcb0fc3
('libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()')
This commit is contained in:

committed by
Thomas Haller

parent
bdcc85de76
commit
8ae9cf4698
@@ -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;
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user