From a35d8ff76984fe9b5342be874909085b2d77a9b9 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 11 Oct 2022 08:35:59 +0200
Subject: [PATCH] cli: don't call setenv() after fork

setenv() cannot be called after fork, because it might allocate memory,
which can deadlock.

Instead, prepare the environment and use execvpe().

`man 2 fork` says:

  After a fork() in a multithreaded program, the child can safely call
  only async-signal-safe functions (see signal-safety(7)) until such time
  as it calls execve(2).

This means, we are quite strongly limited what can be done in the child
process, before exec. setenv() is not listed as async-signal-safe, obviously
because it allocates memory, and malloc() isn't async-signal-safe either.

See also glib's documentation of GSpawnChildSetupFunc ([1]) about what
can be done in the child process.

[1] https://gitlab.gnome.org/GNOME/glib/-/blob/08cb200aec399b38f0cd825340a71fe6caadfb1b/glib/gspawn.h#L124
---
 src/nmcli/utils.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/nmcli/utils.c b/src/nmcli/utils.c
index 2342449e38..07b778ff3f 100644
--- a/src/nmcli/utils.c
+++ b/src/nmcli/utils.c
@@ -1447,11 +1447,12 @@ pager_fallback(void)
 pid_t
 nmc_terminal_spawn_pager(const NmcConfig *nmc_config)
 {
-    const char *pager = getenv("PAGER");
-    pid_t       pager_pid;
-    pid_t       parent_pid;
-    int         fd[2];
-    int         errsv;
+    const char        *pager = getenv("PAGER");
+    pid_t              pager_pid;
+    pid_t              parent_pid;
+    int                fd[2];
+    int                errsv;
+    gs_strfreev char **ev = NULL;
 
     if (nmc_config->in_editor || nmc_config->print_output == NMC_PRINT_TERSE
         || !nmc_config->use_colors || g_strcmp0(pager, "") == 0 || getauxval(AT_SECURE))
@@ -1465,6 +1466,10 @@ nmc_terminal_spawn_pager(const NmcConfig *nmc_config)
 
     parent_pid = getpid();
 
+    ev = g_get_environ();
+    ev = g_environ_setenv(ev, "LESS", "FRSXMK", TRUE);
+    ev = g_environ_setenv(ev, "LESSCHARSET", "utf-8", TRUE);
+
     pager_pid = fork();
     if (pager_pid == -1) {
         errsv = errno;
@@ -1480,9 +1485,6 @@ nmc_terminal_spawn_pager(const NmcConfig *nmc_config)
         nm_close(fd[0]);
         nm_close(fd[1]);
 
-        setenv("LESS", "FRSXMK", 1);
-        setenv("LESSCHARSET", "utf-8", 1);
-
         /* Make sure the pager goes away when the parent dies */
         if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0)
             _exit(EXIT_FAILURE);
@@ -1493,8 +1495,8 @@ nmc_terminal_spawn_pager(const NmcConfig *nmc_config)
             _exit(EXIT_SUCCESS);
 
         if (pager) {
-            execlp(pager, pager, NULL);
-            execl("/bin/sh", "sh", "-c", pager, NULL);
+            execvpe(pager, (char **) NM_MAKE_STRV(pager), ev);
+            execvpe("/bin/sh", (char **) NM_MAKE_STRV("sh", "-c", pager), ev);
         }
 
         /* Debian's alternatives command for pagers is
@@ -1503,10 +1505,10 @@ nmc_terminal_spawn_pager(const NmcConfig *nmc_config)
          * shell script that implements a logic that
          * is similar to this one anyway, but is
          * Debian-specific. */
-        execlp("pager", "pager", NULL);
+        execvpe("pager", (char **) NM_MAKE_STRV("pager"), ev);
 
-        execlp("less", "less", NULL);
-        execlp("more", "more", NULL);
+        execvpe("less", (char **) NM_MAKE_STRV("less"), ev);
+        execvpe("more", (char **) NM_MAKE_STRV("more"), ev);
 
         pager_fallback();
         /* not reached */