Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user namespace bugfixes and features #6865

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ AC_CONFIG_FILES([
tests/zfs-tests/callbacks/Makefile
tests/zfs-tests/cmd/Makefile
tests/zfs-tests/cmd/chg_usr_exec/Makefile
tests/zfs-tests/cmd/user_ns_exec/Makefile
tests/zfs-tests/cmd/devname2devid/Makefile
tests/zfs-tests/cmd/dir_rd_update/Makefile
tests/zfs-tests/cmd/file_check/Makefile
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/cmd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ EXTRA_DIST = file_common.h

SUBDIRS = \
chg_usr_exec \
user_ns_exec \
devname2devid \
dir_rd_update \
file_check \
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/cmd/user_ns_exec/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/user_ns_exec
6 changes: 6 additions & 0 deletions tests/zfs-tests/cmd/user_ns_exec/Makefile.am
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
include $(top_srcdir)/config/Rules.am

pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/bin

pkgexec_PROGRAMS = user_ns_exec
user_ns_exec_SOURCES = user_ns_exec.c
179 changes: 179 additions & 0 deletions tests/zfs-tests/cmd/user_ns_exec/user_ns_exec.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* CDDL HEADER START
*
* The contents of this file are subject to the terms of the
* Common Development and Distribution License (the "License").
* You may not use this file except in compliance with the License.
*
* You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
* or http://www.opensolaris.org/os/licensing.
* See the License for the specific language governing permissions
* and limitations under the License.
*
* When distributing Covered Code, include this CDDL HEADER in each
* file and include the License file at usr/src/OPENSOLARIS.LICENSE.
* If applicable, add the following below this CDDL HEADER, with the
* fields enclosed by brackets "[]" replaced with your own identifying
* information: Portions Copyright [yyyy] [name of copyright owner]
*
* CDDL HEADER END
*/

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <limits.h>
#include <sys/types.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/wait.h>
#include <fcntl.h>
#include <errno.h>
#include <signal.h>
#include <sched.h>

#define EXECSHELL "/bin/sh"
#define UIDMAP "0 100000 65536"

static int
child_main(int argc, char *argv[], int sync_pipe)
{
char sync_buf;
char cmds[BUFSIZ] = { 0 };
char sep[] = " ";
int i, len;

if (unshare(CLONE_NEWUSER | CLONE_NEWNS) != 0) {
perror("unshare");
return (errno);
}

/* tell parent we entered the new namespace */
if (write(sync_pipe, "1", 1) != 1) {
perror("write");
return (errno);
}

/* wait for parent to setup the uid mapping */
if (read(sync_pipe, &sync_buf, 1) != 1) {
(void) fprintf(stderr, "user namespace setup failed\n");
return (EFAULT);
}

close(sync_pipe);

if (setuid(0) != 0) {
perror("setuid");
return (errno);
}
if (setgid(0) != 0) {
perror("setgid");
return (errno);
}

len = 0;
for (i = 1; i < argc; i++) {
(void) snprintf(cmds+len, sizeof (cmds)-len,
"%s%s", argv[i], sep);
len += strlen(argv[i]) + strlen(sep);
}

if (execl(EXECSHELL, "sh", "-c", cmds, (char *)NULL) != 0) {
perror("execl: " EXECSHELL);
return (errno);
}

return (0);
}

static int
set_idmap(pid_t pid, const char *file)
{
int result = 0;
int mapfd;
char path[PATH_MAX];

(void) snprintf(path, sizeof (path), "/proc/%d/%s", (int)pid, file);

mapfd = open(path, O_WRONLY);
if (mapfd < 0) {
perror("open");
return (errno);
}

if (write(mapfd, UIDMAP, sizeof (UIDMAP)-1) != sizeof (UIDMAP)-1) {
perror("write");
result = (errno);
}

close(mapfd);

return (result);
}

int
main(int argc, char *argv[])
{
char sync_buf;
int exit_code, wstatus;
int syncfd[2];
pid_t child;

if (argc < 2 || strlen(argv[1]) == 0) {
(void) printf("\tUsage: %s <commands> ...\n", argv[0]);
return (1);
}

if (socketpair(AF_UNIX, SOCK_STREAM, 0, syncfd) != 0) {
perror("socketpair");
return (errno);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning errno here is dodgy since after a successful call to perror(3) errno is technically undefined. You should save errno in a local variable if you want to use it latter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that, since chg_usr_exec.c is doing the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's not good either and should really be fixed at some point.

}

child = fork();
if (child == (pid_t)-1) {
perror("fork");
return (errno);
}

if (child == 0) {
close(syncfd[0]);
return (child_main(argc, argv, syncfd[1]));
}

close(syncfd[1]);

/* wait for the child to have unshared its namespaces */
if (read(syncfd[0], &sync_buf, 1) != 1) {
perror("read");
goto error_errno;
}

/* write uid mapping */
exit_code = set_idmap(child, "uid_map");
if (exit_code != 0)
goto error;
exit_code = set_idmap(child, "gid_map");
if (exit_code != 0)
goto error;

/* tell the child to proceed */
if (write(syncfd[0], "1", 1) != 1) {
perror("write");
goto error_errno;
}
close(syncfd[0]);
done:
while (waitpid(child, &wstatus, 0) != child) {
/* Keep it simple. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think the comment here is needed.

kill(child, SIGKILL);
}
if (exit_code == 0)
exit_code = WEXITSTATUS(wstatus);

return (exit_code);
error_errno:
exit_code = errno;
error:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above you can't safely assume errno is still valid here. Given that, I think it would be simpler to drop the error* labels and do the needed error handling in each conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that it's just a helper for tests most of the cases don't really need errno anyway and returning 1 would work just as much. A "failed" or "not failed" exit status should suffice after perror() printed the message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

kill(child, SIGKILL);
goto done;
}