All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	libvir-list@redhat.com, "Stefan Weil" <sw@weilnetz.de>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eric Blake" <eblake@redhat.com>
Subject: [PATCH 2/4] os-posix: refactor code handling the -runas argument
Date: Fri,  4 Mar 2022 11:56:55 +0000	[thread overview]
Message-ID: <20220304115657.3177925-3-berrange@redhat.com> (raw)
In-Reply-To: <20220304115657.3177925-1-berrange@redhat.com>

Change the change_process_uid() function so that it takes its input as
parameters instead of relying on static global variables.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 os-posix.c | 83 +++++++++++++++++++++++++-----------------------------
 1 file changed, 39 insertions(+), 44 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 7cd662098e..5a127feee2 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,13 +42,9 @@
 #include <sys/prctl.h>
 #endif
 
-/*
- * Must set all three of these at once.
- * Legal combinations are              unset   by name   by uid
- */
-static struct passwd *user_pwd;    /*   NULL   non-NULL   NULL   */
-static uid_t user_uid = (uid_t)-1; /*   -1      -1        >=0    */
-static gid_t user_gid = (gid_t)-1; /*   -1      -1        >=0    */
+static char *user_name;
+static uid_t user_uid = (uid_t)-1;
+static gid_t user_gid = (gid_t)-1;
 
 static const char *chroot_dir;
 static int daemonize;
@@ -100,7 +96,8 @@ void os_set_proc_name(const char *s)
 }
 
 
-static bool os_parse_runas_uid_gid(const char *optarg)
+static bool os_parse_runas_uid_gid(const char *optarg,
+                                   uid_t *runas_uid, gid_t *runas_gid)
 {
     unsigned long lv;
     const char *ep;
@@ -120,9 +117,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
         return false;
     }
 
-    user_pwd = NULL;
-    user_uid = got_uid;
-    user_gid = got_gid;
+    *runas_uid = got_uid;
+    *runas_gid = got_gid;
     return true;
 }
 
@@ -132,13 +128,18 @@ static bool os_parse_runas_uid_gid(const char *optarg)
  */
 int os_parse_cmd_args(int index, const char *optarg)
 {
+    struct passwd *user_pwd;
+
     switch (index) {
     case QEMU_OPTION_runas:
         user_pwd = getpwnam(optarg);
         if (user_pwd) {
-            user_uid = -1;
-            user_gid = -1;
-        } else if (!os_parse_runas_uid_gid(optarg)) {
+            user_uid = user_pwd->pw_uid;
+            user_gid = user_pwd->pw_gid;
+            user_name = g_strdup(user_pwd->pw_name);
+        } else if (!os_parse_runas_uid_gid(optarg,
+                                           &user_uid,
+                                           &user_gid)) {
             error_report("User \"%s\" doesn't exist"
                          " (and is not <uid>:<gid>)",
                          optarg);
@@ -158,41 +159,33 @@ int os_parse_cmd_args(int index, const char *optarg)
     return 0;
 }
 
-static void change_process_uid(void)
+static void change_process_uid(uid_t uid, gid_t gid, const char *name)
 {
-    assert((user_uid == (uid_t)-1) || user_pwd == NULL);
-    assert((user_uid == (uid_t)-1) ==
-           (user_gid == (gid_t)-1));
-
-    if (user_pwd || user_uid != (uid_t)-1) {
-        gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
-        uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
-        if (setgid(intended_gid) < 0) {
-            error_report("Failed to setgid(%d)", intended_gid);
-            exit(1);
-        }
-        if (user_pwd) {
-            if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-                error_report("Failed to initgroups(\"%s\", %d)",
-                        user_pwd->pw_name, user_pwd->pw_gid);
-                exit(1);
-            }
-        } else {
-            if (setgroups(1, &user_gid) < 0) {
-                error_report("Failed to setgroups(1, [%d])",
-                        user_gid);
-                exit(1);
-            }
-        }
-        if (setuid(intended_uid) < 0) {
-            error_report("Failed to setuid(%d)", intended_uid);
+    if (setgid(gid) < 0) {
+        error_report("Failed to setgid(%d)", gid);
+        exit(1);
+    }
+    if (name) {
+        if (initgroups(name, gid) < 0) {
+            error_report("Failed to initgroups(\"%s\", %d)",
+                         name, gid);
             exit(1);
         }
-        if (setuid(0) != -1) {
-            error_report("Dropping privileges failed");
+    } else {
+        if (setgroups(1, &gid) < 0) {
+            error_report("Failed to setgroups(1, [%d])",
+                         gid);
             exit(1);
         }
     }
+    if (setuid(uid) < 0) {
+        error_report("Failed to setuid(%d)", uid);
+        exit(1);
+    }
+    if (setuid(0) != -1) {
+        error_report("Dropping privileges failed");
+        exit(1);
+    }
 }
 
 static void change_root(void)
@@ -275,7 +268,9 @@ void os_setup_post(void)
     }
 
     change_root();
-    change_process_uid();
+    if (user_uid != -1 && user_gid != -1) {
+        change_process_uid(user_uid, user_gid, user_name);
+    }
 
     if (daemonize) {
         uint8_t status = 0;
-- 
2.34.1



  parent reply	other threads:[~2022-03-04 12:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-04 11:56 [PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize Daniel P. Berrangé
2022-03-04 11:56 ` [PATCH 1/4] softmmu: remove deprecated --enable-fips option Daniel P. Berrangé
2022-03-04 13:55   ` Philippe Mathieu-Daudé
2022-03-04 17:14   ` Eric Blake
2022-03-04 11:56 ` Daniel P. Berrangé [this message]
2022-03-04 17:19   ` [PATCH 2/4] os-posix: refactor code handling the -runas argument Eric Blake
2022-03-04 11:56 ` [PATCH 3/4] os-posix: refactor code handling the -chroot argument Daniel P. Berrangé
2022-03-04 13:54   ` Philippe Mathieu-Daudé
2022-03-04 11:56 ` [PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code Daniel P. Berrangé
2022-03-04 14:54   ` Daniel P. Berrangé
2022-03-04 17:21     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220304115657.3177925-3-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sw@weilnetz.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.