Skip to content

Commit

Permalink
Improve diagnostics for lookup_{user,group}
Browse files Browse the repository at this point in the history
Make sure to pass error codes back to callers and include them in
diagnostics printed by minijail0.

Bug: chromium:1055067
Test: Compiles and passes tests.

Change-Id: Ic07c5917e7826dd7cf2bfbd3483ad97ad199b670
  • Loading branch information
mnissler authored and Treehugger Robot committed Feb 26, 2020
1 parent 0bb824a commit 160d58f
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 18 deletions.
26 changes: 14 additions & 12 deletions minijail0_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,16 @@ static void set_user(struct minijail *j, const char *arg, uid_t *out_uid,
return;
}

if (lookup_user(arg, out_uid, out_gid)) {
fprintf(stderr, "Bad user: '%s'\n", arg);
int ret = lookup_user(arg, out_uid, out_gid);
if (ret) {
fprintf(stderr, "Bad user '%s': %s\n", arg, strerror(-ret));
exit(1);
}

if (minijail_change_user(j, arg)) {
fprintf(stderr, "Bad user: '%s'\n", arg);
ret = minijail_change_user(j, arg);
if (ret) {
fprintf(stderr, "minijail_change_user('%s') failed: %s\n", arg,
strerror(-ret));
exit(1);
}
}
Expand All @@ -61,15 +64,13 @@ static void set_group(struct minijail *j, const char *arg, gid_t *out_gid)
return;
}

if (lookup_group(arg, out_gid)) {
fprintf(stderr, "Bad group: '%s'\n", arg);
int ret = lookup_group(arg, out_gid);
if (ret) {
fprintf(stderr, "Bad group '%s': %s\n", arg, strerror(-ret));
exit(1);
}

if (minijail_change_group(j, arg)) {
fprintf(stderr, "Bad group: '%s'\n", arg);
exit(1);
}
minijail_change_gid(j, *out_gid);
}

/*
Expand All @@ -81,15 +82,16 @@ static void suppl_group_add(size_t *suppl_gids_count, gid_t **suppl_gids,
char *end = NULL;
int groupid = strtod(arg, &end);
gid_t gid;
int ret;
if (!*end && *arg) {
/* A gid number has been specified, proceed. */
gid = groupid;
} else if (lookup_group(arg, &gid)) {
} else if ((ret = lookup_group(arg, &gid))) {
/*
* A group name has been specified,
* but doesn't exist: we bail out.
*/
fprintf(stderr, "Bad group: '%s'\n", arg);
fprintf(stderr, "Bad group '%s': %s\n", arg, strerror(-ret));
exit(1);
}

Expand Down
17 changes: 11 additions & 6 deletions system.c
Original file line number Diff line number Diff line change
Expand Up @@ -395,17 +395,20 @@ int lookup_user(const char *user, uid_t *uid, gid_t *gid)
buf = malloc(sz);
if (!buf)
return -ENOMEM;
getpwnam_r(user, &pw, buf, sz, &ppw);

int ret = getpwnam_r(user, &pw, buf, sz, &ppw);
/*
* We're safe to free the buffer here. The strings inside |pw| point
* inside |buf|, but we don't use any of them; this leaves the pointers
* dangling but it's safe. |ppw| points at |pw| if getpwnam_r(3)
* succeeded.
*/
free(buf);
/* getpwnam_r(3) does *not* set errno when |ppw| is NULL. */

if (ret != 0)
return -ret; /* Error */
if (!ppw)
return -1;
return -ENOENT; /* Not found */

*uid = ppw->pw_uid;
*gid = ppw->pw_gid;
Expand All @@ -431,16 +434,18 @@ int lookup_group(const char *group, gid_t *gid)
buf = malloc(sz);
if (!buf)
return -ENOMEM;
getgrnam_r(group, &gr, buf, sz, &pgr);
int ret = getgrnam_r(group, &gr, buf, sz, &pgr);
/*
* We're safe to free the buffer here. The strings inside gr point
* inside buf, but we don't use any of them; this leaves the pointers
* dangling but it's safe. pgr points at gr if getgrnam_r succeeded.
*/
free(buf);
/* getgrnam_r(3) does *not* set errno when |pgr| is NULL. */

if (ret != 0)
return -ret; /* Error */
if (!pgr)
return -1;
return -ENOENT; /* Not found */

*gid = pgr->gr_gid;
return 0;
Expand Down

0 comments on commit 160d58f

Please sign in to comment.