Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Jun 2021 20:24:00 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: ba5ed8109cc9 - releng/13.0 - libcasper: fix descriptors numbers
Message-ID:  <202106292024.15TKO0dh006958@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch releng/13.0 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=ba5ed8109cc99cccd4eebb626225afc5eefa16b0

commit ba5ed8109cc99cccd4eebb626225afc5eefa16b0
Author:     Mariusz Zaborski <oshogbo@FreeBSD.org>
AuthorDate: 2021-06-09 21:46:51 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-06-29 17:09:02 +0000

    libcasper: fix descriptors numbers
    
    Casper services expect that the first 3 descriptors (stdin/stdout/stderr)
    will point to /dev/null. Which Casper will ensure later. The Casper
    services are forked from the original process. If the initial process
    closes one of those descriptors, Casper may reuse one of them for it on
    purpose. If this is the case, then renumarate the descriptors used by
    Casper to higher numbers. This is done already after the fork, so it
    doesn't break the parent process.
    
    Approved by:    so
    Security:       EN-21:19.libcasper
    PR:             255339
    Reported by:    Borja Marcos <borjam (at) sarenet.es>
    Tested by:      jkim@
    
    (cherry picked from commit aa310ebfba3d49a0b6b03a103b969731a8136a73)
    (cherry picked from commit 934e10b4a388b13c2bcd8fbac8cd8cc4a641b1b0)
---
 lib/libcasper/libcasper/libcasper_impl.c | 27 +++++++++++++++++++++++++++
 lib/libcasper/libcasper/libcasper_impl.h |  1 +
 lib/libcasper/libcasper/service.c        | 23 +++++++++++++----------
 lib/libcasper/libcasper/zygote.c         | 15 +++++++++------
 4 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/lib/libcasper/libcasper/libcasper_impl.c b/lib/libcasper/libcasper/libcasper_impl.c
index e4411630c8a1..ae28c8769a07 100644
--- a/lib/libcasper/libcasper/libcasper_impl.c
+++ b/lib/libcasper/libcasper/libcasper_impl.c
@@ -32,8 +32,10 @@
  * $FreeBSD$
  */
 
+#include <err.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <paths.h>
 #include <stdlib.h>
 
 #include "libcasper_impl.h"
@@ -44,3 +46,28 @@ fd_is_valid(int fd)
 
 	return (fcntl(fd, F_GETFL) != -1 || errno != EBADF);
 }
+
+void
+fd_fix_environment(int *fdp)
+{
+	int nullfd, nfd;
+
+	if (*fdp > STDERR_FILENO)
+		return;
+
+	nullfd = open(_PATH_DEVNULL, O_RDWR);
+	if (nullfd == -1)
+		errx(1, "Unable to open %s", _PATH_DEVNULL);
+
+	while (*fdp <= STDERR_FILENO) {
+		nfd = dup(*fdp);
+		if (nfd == -1)
+			errx(1, "Unable to secure fd");
+		if (dup2(nullfd, *fdp) == -1)
+			errx(1, "Unable to secure fd");
+		*fdp = nfd;
+	}
+
+	close(nullfd);
+}
+
diff --git a/lib/libcasper/libcasper/libcasper_impl.h b/lib/libcasper/libcasper/libcasper_impl.h
index 11e43f083977..24049a0c07c8 100644
--- a/lib/libcasper/libcasper/libcasper_impl.h
+++ b/lib/libcasper/libcasper/libcasper_impl.h
@@ -44,6 +44,7 @@ struct service;
 struct service_connection;
 
 bool fd_is_valid(int fd);
+void fd_fix_environment(int *fdp);
 
 /* Private service functions. */
 struct service	*service_alloc(const char *name,
diff --git a/lib/libcasper/libcasper/service.c b/lib/libcasper/libcasper/service.c
index 5c1c64d9a9d7..e87d0640347c 100644
--- a/lib/libcasper/libcasper/service.c
+++ b/lib/libcasper/libcasper/service.c
@@ -386,24 +386,27 @@ stdnull(void)
 }
 
 static void
-service_clean(int sock, int procfd, uint64_t flags)
+service_clean(int *sockp, int *procfdp, uint64_t flags)
 {
 	int fd, maxfd, minfd;
 
-	assert(sock > STDERR_FILENO);
-	assert(procfd > STDERR_FILENO);
-	assert(sock != procfd);
+	fd_fix_environment(sockp);
+	fd_fix_environment(procfdp);
+
+	assert(*sockp > STDERR_FILENO);
+	assert(*procfdp > STDERR_FILENO);
+	assert(*sockp != *procfdp);
 
 	if ((flags & CASPER_SERVICE_STDIO) == 0)
 		stdnull();
 
 	if ((flags & CASPER_SERVICE_FD) == 0) {
-		if (procfd > sock) {
-			maxfd = procfd;
-			minfd = sock;
+		if (*procfdp > *sockp) {
+			maxfd = *procfdp;
+			minfd = *sockp;
 		} else {
-			maxfd = sock;
-			minfd = procfd;
+			maxfd = *sockp;
+			minfd = *procfdp;
 		}
 
 		for (fd = STDERR_FILENO + 1; fd < maxfd; fd++) {
@@ -424,7 +427,7 @@ service_start(struct service *service, int sock, int procfd)
 	assert(service != NULL);
 	assert(service->s_magic == SERVICE_MAGIC);
 	setproctitle("%s", service->s_name);
-	service_clean(sock, procfd, service->s_flags);
+	service_clean(&sock, &procfd, service->s_flags);
 
 	if (service_connection_add(service, sock, NULL) == NULL)
 		_exit(1);
diff --git a/lib/libcasper/libcasper/zygote.c b/lib/libcasper/libcasper/zygote.c
index 2b84bb49a695..5cdd139cc134 100644
--- a/lib/libcasper/libcasper/zygote.c
+++ b/lib/libcasper/libcasper/zygote.c
@@ -52,6 +52,7 @@ __FBSDID("$FreeBSD$");
 #include <strings.h>
 #include <unistd.h>
 
+#include "libcasper_impl.h"
 #include "zygote.h"
 
 /* Zygote info. */
@@ -104,7 +105,7 @@ zygote_clone_service_execute(int *chanfdp, int *procfdp)
  * between sandbox and its owner.
  */
 static void
-zygote_main(int sock)
+zygote_main(int *sockp)
 {
 	int error, procfd;
 	int chanfd[2];
@@ -113,12 +114,14 @@ zygote_main(int sock)
 	zygote_func_t *func;
 	pid_t pid;
 
-	assert(sock > STDERR_FILENO);
+	fd_fix_environment(sockp);
+
+	assert(*sockp > STDERR_FILENO);
 
 	setproctitle("zygote");
 
 	for (;;) {
-		nvlin = nvlist_recv(sock, 0);
+		nvlin = nvlist_recv(*sockp, 0);
 		if (nvlin == NULL) {
 			if (errno == ENOTCONN) {
 				/* Casper exited. */
@@ -157,7 +160,7 @@ zygote_main(int sock)
 			break;
 		case 0:
 			/* Child. */
-			close(sock);
+			close(*sockp);
 			close(chanfd[0]);
 			func(chanfd[1]);
 			/* NOTREACHED */
@@ -179,7 +182,7 @@ send:
 			nvlist_move_descriptor(nvlout, "chanfd", chanfd[0]);
 			nvlist_move_descriptor(nvlout, "procfd", procfd);
 		}
-		(void)nvlist_send(sock, nvlout);
+		(void)nvlist_send(*sockp, nvlout);
 		nvlist_destroy(nvlout);
 	}
 	/* NOTREACHED */
@@ -206,7 +209,7 @@ zygote_init(void)
 	case 0:
 		/* Child. */
 		close(sp[0]);
-		zygote_main(sp[1]);
+		zygote_main(&sp[1]);
 		/* NOTREACHED */
 		abort();
 	default:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106292024.15TKO0dh006958>