From 56b8c61a15e48d82168773bf97aaa7811eaac721 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Tue, 22 Aug 2017 17:15:23 +0800 Subject: [PATCH 1/5] set ngroups=10 to avoid unnecessary realloc() later Signed-off-by: Lai Jiangshan --- src/exec.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/exec.c b/src/exec.c index 923cd252..c348de7e 100644 --- a/src/exec.c +++ b/src/exec.c @@ -220,9 +220,11 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) } uid = pwd->pw_uid; gid = pwd->pw_gid; + fprintf(stdout, "found the user: %s, uid:%d, gid:%d\n", user, uid, gid); // get groups of user - groups = malloc(sizeof(gid_t) * 10); + ngroups = 10; + groups = malloc(sizeof(gid_t) * ngroups); if (groups == NULL) { goto fail; } @@ -236,6 +238,7 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) goto fail; } } + fprintf(stdout, "get %d groups from /etc/group\n", ngroups); // set user related envs. the container env config can overwrite it setenv("USER", pwd->pw_name, 1); From 0ab4f298dca3bd96574ffde4c59083b972e2ef06 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Tue, 22 Aug 2017 17:16:50 +0800 Subject: [PATCH 2/5] check arguments before syscall Signed-off-by: Lai Jiangshan --- src/exec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/exec.c b/src/exec.c index c348de7e..d3cbeee2 100644 --- a/src/exec.c +++ b/src/exec.c @@ -308,15 +308,15 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) } // apply - if (groups && setgroups(ngroups, groups) < 0) { + if (ngroups > 0 && setgroups(ngroups, groups) < 0) { perror("setgroups() fails"); goto fail; } - if (setgid(gid) < 0) { + if (gid > 0 && setgid(gid) < 0) { perror("setgid() fails"); goto fail; } - if (setuid(uid) < 0) { + if (uid > 0 && setuid(uid) < 0) { perror("setuid() fails"); goto fail; } From 8872c65a12610ab01502e8220ebc7838d9041ef5 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Tue, 22 Aug 2017 17:24:50 +0800 Subject: [PATCH 3/5] fix offset-by-one bug in hyper_getgrouplist() Signed-off-by: Lai Jiangshan --- src/util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/util.c b/src/util.c index 8b4fa195..96a72695 100644 --- a/src/util.c +++ b/src/util.c @@ -209,7 +209,8 @@ int hyper_getgrouplist(const char *user, gid_t group, gid_t *groups, int *ngroup int j; for (j = 0; gr->gr_mem && gr->gr_mem[j]; j++) { if (!strcmp(gr->gr_mem[j], user)) { - if (nr + 1 < *ngroups) + fprintf(stdout, "hyper_getgrouplist() found matched group for user %s, grname: %s, gid: %d\n", user, gr->gr_name, gr->gr_gid); + if (nr < *ngroups) groups[nr] = gr->gr_gid; nr++; } @@ -217,7 +218,8 @@ int hyper_getgrouplist(const char *user, gid_t group, gid_t *groups, int *ngroup } fclose(file); if (nr == 0) { - if (nr + 1 < *ngroups) + fprintf(stdout, "hyper_getgrouplist() adds the default group to list, gid:%d\n", group); + if (nr < *ngroups) groups[nr] = group; nr++; } From 4845347592457eacda161711ee25812b7b2f5b46 Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Tue, 22 Aug 2017 17:40:57 +0800 Subject: [PATCH 4/5] always parse the user/groups from /etc/passwd and /etc/group Signed-off-by: Lai Jiangshan --- src/exec.c | 39 +++++++++------------------------------ 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/exec.c b/src/exec.c index d3cbeee2..3faa71a1 100644 --- a/src/exec.c +++ b/src/exec.c @@ -192,7 +192,7 @@ struct hyper_event_ops err_ops = { static int hyper_setup_exec_user(struct hyper_exec *exec) { - char *user = exec->user == NULL || strlen(exec->user) == 0 ? NULL : exec->user; + char *user = exec->user == NULL || strlen(exec->user) == 0 ? "0" : exec->user; char *group = exec->group == NULL || strlen(exec->group) == 0 ? NULL : exec->group; uid_t uid = 0; @@ -200,24 +200,10 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) int ngroups = 0; gid_t *reallocgroups, *groups = NULL; - // check the config - if (!user && !group && exec->nr_additional_groups == 0) { - return 0; - } - // get uid - if (user) { - fprintf(stdout, "try to find the user: %s\n", user); - struct passwd *pwd = hyper_getpwnam(user); - if (pwd == NULL) { - unsigned long id; - if (!hyper_name_to_id(user, &id)) { - perror("can't find the user"); - return -1; - } - uid = id; - goto get_gid; - } + fprintf(stdout, "try to find the user(or uid): %s\n", user); + struct passwd *pwd = hyper_getpwnam(user); + if (pwd != NULL) { uid = pwd->pw_uid; gid = pwd->pw_gid; fprintf(stdout, "found the user: %s, uid:%d, gid:%d\n", user, uid, gid); @@ -244,21 +230,14 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) setenv("USER", pwd->pw_name, 1); setenv("HOME", pwd->pw_dir, 1); } else { - ngroups = getgroups(0, NULL); - if (ngroups < 0) { - goto fail; - } - groups = malloc(sizeof(gid_t) * ngroups); - if (groups == NULL) { - goto fail; - } - ngroups = getgroups(ngroups, groups); - if (ngroups < 0) { - goto fail; + unsigned long id; + if (!hyper_name_to_id(user, &id)) { + perror("can't find the user"); + return -1; } + uid = id; } -get_gid: // get gid if (group) { fprintf(stdout, "try to find the group: %s\n", group); From 4ed10eec59bf2c6552fc230b3b686fa2f2682fdb Mon Sep 17 00:00:00 2001 From: Lai Jiangshan Date: Tue, 22 Aug 2017 18:29:03 +0800 Subject: [PATCH 5/5] always set the group of the pts to tty Signed-off-by: Lai Jiangshan --- src/exec.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/exec.c b/src/exec.c index 3faa71a1..7faf092b 100644 --- a/src/exec.c +++ b/src/exec.c @@ -278,9 +278,15 @@ static int hyper_setup_exec_user(struct hyper_exec *exec) // setup the owner of tty if (exec->tty) { + gid_t tty_gid = gid; char ptmx[512]; sprintf(ptmx, "/dev/pts/%d", exec->ptyno); - if (chown(ptmx, uid, gid) < 0) { + + struct group *gr = hyper_getgrnam("tty"); + if (gr != NULL) { + tty_gid = gr->gr_gid; + } + if (chown(ptmx, uid, tty_gid) < 0) { perror("failed to change the owner for the slave pty file"); goto fail; }