Use vfork()/execvp() instead of system().
authorgraham.gower <graham.gower@e8e0d7a0-c8d9-11dd-a880-a1081c7ac358>
Tue, 17 Nov 2009 00:17:55 +0000 (00:17 +0000)
committergraham.gower <graham.gower@e8e0d7a0-c8d9-11dd-a880-a1081c7ac358>
Tue, 17 Nov 2009 00:17:55 +0000 (00:17 +0000)
Parts based on a patch by Mike Westerhof for OpenEmbedded.

git-svn-id: http://opkg.googlecode.com/svn/trunk@320 e8e0d7a0-c8d9-11dd-a880-a1081c7ac358

libopkg/opkg_cmd.c
libopkg/opkg_conf.c
libopkg/opkg_download.c
libopkg/opkg_install.c
libopkg/pkg.c
libopkg/xsystem.c
libopkg/xsystem.h

index b86e670..a93483f 100644 (file)
@@ -340,7 +340,8 @@ static int opkg_finalize_intercepts(opkg_intercept_t ctx)
            
            sprintf_alloc (&path, "%s/%s", ctx->statedir, de->d_name);
            if (access (path, X_OK) == 0) {
-               xsystem (path);
+               const char *argv[] = {"sh", "-c", path, NULL};
+               xsystem (argv);
            }
            free (path);
        }
index bd73317..cade51a 100644 (file)
@@ -25,7 +25,6 @@
 #include "opkg_message.h"
 #include "file_util.h"
 #include "str_util.h"
-#include "xsystem.h"
 #include "opkg_defines.h"
 #include "libbb/libbb.h"
 
index be3ae2a..953627b 100644 (file)
@@ -147,16 +147,21 @@ int opkg_download(opkg_conf_t *conf, const char *src,
 #else
     {
       int res;
-      char *wgetcmd;
-      char *wgetopts;
-      wgetopts = getenv("OPKG_WGETOPTS");
-      sprintf_alloc(&wgetcmd, "wget -q %s%s -O \"%s\" \"%s\"",
-                   (conf->http_proxy || conf->ftp_proxy) ? "-Y on " : "",
-                   (wgetopts!=NULL) ? wgetopts : "",
-                   tmp_file_location, src);
-      opkg_message(conf, OPKG_INFO, "Executing: %s\n", wgetcmd);
-      res = xsystem(wgetcmd);
-      free(wgetcmd);
+      const char *argv[8];
+      int i = 0;
+
+      argv[i++] = "wget";
+      argv[i++] = "-q";
+      if (conf->http_proxy || conf->ftp_proxy) {
+       argv[i++] = "-Y";
+       argv[i++] = "on";
+      }
+      argv[i++] = "-O";
+      argv[i++] = tmp_file_location;
+      argv[i++] = src;
+      argv[i++] = NULL;
+      res = xsystem(argv);
+
       if (res) {
        opkg_message(conf, OPKG_ERROR, "Failed to download %s, error %d\n", src, res);
        free(tmp_file_location);
index 02a2be4..5e8596b 100644 (file)
@@ -1610,13 +1610,8 @@ static int user_prefers_old_conffile(const char *file_name, const char *backup)
          }
 
          if (strcmp(response, "d") == 0) {
-              char *cmd;
-
-              free(response);
-              /* XXX: BUG rewrite to use exec or busybox's internal diff */
-              sprintf_alloc(&cmd, "diff -u %s %s", backup, file_name);
-              xsystem(cmd);
-              free(cmd);
+              const char *argv[] = {"diff", "-u", backup, file_name, NULL};
+              xsystem(argv);
               printf("    [Press ENTER to continue]\n");
               response = file_read_line_alloc(stdin);
               free(response);
index 78b6cad..b0c2b69 100644 (file)
@@ -1195,8 +1195,10 @@ int pkg_run_script(opkg_conf_t *conf, pkg_t *pkg,
 
      sprintf_alloc(&cmd, "%s %s", path, args);
      free(path);
-
-     err = xsystem(cmd);
+     {
+         const char *argv[] = {"sh", "-c", cmd, NULL};
+         err = xsystem(argv);
+     }
      free(cmd);
 
      if (err) {
index 267f2f9..123510f 100644 (file)
 #include <sys/wait.h>
 
 #include "xsystem.h"
-
-/* XXX: FEATURE: I shouldn't actually use system(3) at all. I don't
-   really need the /bin/sh invocation which takes resources and
-   introduces security problems. I should switch all of this to a sort
-   of execl() or execv() interface/implementation.
-*/
+#include "libbb/libbb.h"
 
 /* Like system(3), but with error messages printed if the fork fails
    or if the child process dies due to an uncaught signal. Also, the
    Otherwise, the 8-bit return value of the program ala WEXITSTATUS
    as defined in <sys/wait.h>.
 */
-int xsystem(const char *cmd)
+int
+xsystem(const char *argv[])
 {
-    int err;
+       int status;
+       pid_t pid;
+
+       pid = vfork();
 
-    err = system(cmd);
+       switch (pid) {
+       case -1:
+               perror_msg("%s: %s: vfork", __FUNCTION__, argv[0]);
+               return -1;
+       case 0:
+               /* child */
+               execvp(argv[0], (char*const*)argv);
+               _exit(-1);
+       default:
+               /* parent */
+               break;
+       }
 
-    if (err == -1) {
-       fprintf(stderr, "%s: ERROR: fork failed before execution: `%s'\n",
-               __FUNCTION__, cmd);
-       return -1;
-    }
+       if (waitpid(pid, &status, 0) == -1) {
+               perror_msg("%s: %s: waitpid", __FUNCTION__, argv[0]);
+               return -1;
+       }
 
-    if (WIFSIGNALED(err)) {
-       fprintf(stderr, "%s: ERROR: Child process died due to signal %d: `%s'\n",
-               __FUNCTION__, WTERMSIG(err), cmd);
-       return -1;
-    }
+       if (WIFSIGNALED(status)) {
+               error_msg("%s: %s: Child killed by signal %d\n",
+                       __FUNCTION__, argv[0], WTERMSIG(status));
+               return -1;
+       }
 
-    if (WIFEXITED(err)) {
-       /* Normal child exit */
-       return WEXITSTATUS(err);
-    }
+       if (!WIFEXITED(status)) {
+               /* shouldn't happen */
+               error_msg("%s: %s: Your system is broken: got status %d "
+                       "from waitpid\n", __FUNCTION__, argv[0], status);
+               return -1;
+       }
 
-    fprintf(stderr, "%s: ERROR: Received unintelligible return value from system: %d",
-           __FUNCTION__, err);
-    return -1;
+       return WEXITSTATUS(status);
 }
-        
index cc1ca2a..3c7a3d2 100644 (file)
@@ -28,7 +28,7 @@
    Otherwise, the 8-bit return value of the program ala WEXITSTATUS
    as defined in <sys/wait.h>.
 */
-int xsystem(const char *cmd);
+int xsystem(const char *argv[]);
 
 #endif