Download raw body.
[PATCH]: Add POSIX O_CLOFORK flag
Illumos updated their implementation and also the test code.
https://github.com/illumos/illumos-gate/commit/b3ff81dc6673bee7f291d9d66a832cb3e1004f49
Attached patch to the tests. This is now simplified and we just call
fork() instead of simulating Solaris forkx as there's no need for that.
These tests pass. I think we're good to go.
Best,
Ricardo
On 6/25/25 12:09 PM, Theo Buehler wrote:
>> How can I do this? Extending current tests is not worth it, imo.
> That would look something like this.
>
> Attached is a tarball for a new port, illumos-os-tests.tgz, to be
> extracted in /usr/ports/devel. It bundles the CDDL and all the stuff
> beneath os-tests in illumos-gate with the patches from your gist applied
> into a port installing these files in /usr/local/share/illumos-os-tests.
>
> I also attached a package, illumos-os-tests-20250625.tgz, which you can
> install as root with
>
> # env TRUSTED_PKG_PATH=/path/to/tgz pkg_add illumos-os-tests
>
> so you don't need to fiddle with the ports tree yourself.
>
> This part of the kern_descrip.c diff from your
> 2bd21db4e48d499abaac013009cda6d0769e0049 doesn't compile:
>
> case F_SETFD:
> fdplock(fdp);
> - if ((long)SCARG(uap, arg) & 1)
> - fdp->fd_ofileflags[fd] |= UF_EXCLOSE;
> - else
> - fdp->fd_ofileflags[fd] &= ~UF_EXCLOSE;
> + fdp->fd_ofileflags[fd] =
> + (fdp->fd_ofileflags[fd] & ~(UF_EXCLOSE | UF_FORKCLOSE)) |
> + ((SCARG(uap, arg) & FD_CLOEXEC) ? UF_EXCLOSE : 0) |
> + ((SCARG(uap, arg) & FD_CLOFORK) ? UF_FORKCLOSE : 0);
> fdpunlock(fdp);
> break;
>
> /sys/kern/kern_descrip.c:470:25: error: invalid operands to binary expression ('void *' and 'int')
> 470 | ((SCARG(uap, arg) & FD_CLOEXEC) ? UF_EXCLOSE : 0) |
> | ~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~
>
> I assume the two SCARG should be (long)SCARG to match what was removed.
>
>> These tests are more than enough... Still waiting for a response if
>> they can relicense them to BSD though.
> Finally, here's a diff for the regress tests. With the (long)SCARG
> change applied to your diff and with the illumos-os-tests package
> installed, these tests compile and pass on my amd64 laptop. If the
> package isn't installed, you get
>
> ===> illumos
> ===> illumos/oclo
> package illumos-os-tests is required for this regress
> SKIPPED
>
> commit db54671d22ce4adf21cb47b7201bb3633526d24f
> Author: Theo Buehler <tb@openbsd.org>
> Date: Wed Jun 25 11:10:19 2025 +0200
>
> Hook illumos-os-tests for oclo to libc regress
>
> Based on work by Ricardo Branco
>
> Change-Id: I6a6a6964af35f8d5c9eaeb3606b26b88b2ff2d19
>
> diff --git a/regress/lib/libc/Makefile b/regress/lib/libc/Makefile
> index 59d043c62fe..8bdc7809717 100644
> --- a/regress/lib/libc/Makefile
> +++ b/regress/lib/libc/Makefile
> @@ -11,7 +11,7 @@ SUBDIR+= ffs fmemopen fnmatch fpclassify fread
> SUBDIR+= gcvt getaddrinfo getcap getopt getopt_long glob
> SUBDIR+= hash
> SUBDIR+= hsearch
> -SUBDIR+= ieeefp ifnameindex
> +SUBDIR+= ieeefp ifnameindex illumos
> SUBDIR+= ldexp locale longjmp
> SUBDIR+= malloc mkstemp modf
> SUBDIR+= netdb
> diff --git a/regress/lib/libc/illumos/Makefile b/regress/lib/libc/illumos/Makefile
> new file mode 100644
> index 00000000000..7fdceb00349
> --- /dev/null
> +++ b/regress/lib/libc/illumos/Makefile
> @@ -0,0 +1,7 @@
> +# $OpenBSD$
> +
> +SUBDIR += oclo
> +
> +install:
> +
> +.include <bsd.subdir.mk>
> diff --git a/regress/lib/libc/illumos/Makefile.inc b/regress/lib/libc/illumos/Makefile.inc
> new file mode 100644
> index 00000000000..574d9f3d970
> --- /dev/null
> +++ b/regress/lib/libc/illumos/Makefile.inc
> @@ -0,0 +1,7 @@
> +ILLUMOS_OS_TESTDIR = /usr/local/share/illumos-os-tests
> +
> +.if !exists(${ILLUMOS_OS_TESTDIR})
> +regress:
> + @echo package illumos-os-tests is required for this regress
> + @echo SKIPPED
> +.endif
> diff --git a/regress/lib/libc/illumos/oclo/Makefile b/regress/lib/libc/illumos/oclo/Makefile
> new file mode 100644
> index 00000000000..d808c54ab2a
> --- /dev/null
> +++ b/regress/lib/libc/illumos/oclo/Makefile
> @@ -0,0 +1,14 @@
> +# $OpenBSD$
> +
> +.if exists(/usr/local/share/illumos-os-tests)
> +
> +PROGS = oclo
> +PROGS += oclo_errors
> +PROGS += ocloexec_verify
> +
> +LDADD_ocloexec_verify = -lkvm
> +
> +.PATH: /usr/local/share/illumos-os-tests/tests/oclo
> +.endif
> +
> +.include <bsd.regress.mk>
From 3e406f71425b4722515a506c31df266496de4094 Mon Sep 17 00:00:00 2001
From: Ricardo Branco <rbranco@suse.de>
Date: Mon, 30 Jun 2025 09:24:21 +0200
Subject: [PATCH] Adapt oclo tests to OpenBSD
---
oclo.c | 96 +++++++++++++++++++++++++++++++----------------
oclo_errors.c | 29 +++++++++-----
ocloexec_verify.c | 54 ++++++++++++++++++--------
3 files changed, 121 insertions(+), 58 deletions(-)
diff --git a/oclo.c b/oclo.c
index b21c253..7331c23 100644
--- a/oclo.c
+++ b/oclo.c
@@ -45,21 +45,27 @@
* with the divergence of other implementations.
*/
-#include <stdlib.h>
-#include <unistd.h>
-#include <stdbool.h>
-#include <err.h>
-#include <sys/types.h>
+#include <sys/param.h>
+#include <sys/sysctl.h>
#include <sys/stat.h>
-#include <fcntl.h>
-#include <sys/sysmacros.h>
-#include <sys/fork.h>
-#include <wait.h>
+#include <sys/wait.h>
+
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <err.h>
#include <errno.h>
-#include <string.h>
+#include <fcntl.h>
#include <limits.h>
-#include <libgen.h>
-#include <sys/socket.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#define strerrorname_np(e) (sys_errlist[e])
/*
* Verification program name.
@@ -93,8 +99,8 @@ typedef struct clo_rtdata {
} clo_rtdata_t;
static clo_rtdata_t *oclo_rtdata;
-size_t oclo_rtdata_nents = 0;
-size_t oclo_rtdata_next = 0;
+static size_t oclo_rtdata_nents = 0;
+static size_t oclo_rtdata_next = 0;
static int oclo_nextfd = STDERR_FILENO + 1;
static bool
@@ -265,14 +271,24 @@ oclo_fdup_common(const clo_create_t *c, int targ_flags, int cmd)
case F_DUPFD_CLOFORK:
dup = fcntl(fd, cmd, fd);
break;
+#ifdef F_DUP2FD
case F_DUP2FD:
+#endif
+#ifdef F_DUP2FD_CLOEXEC
case F_DUP2FD_CLOEXEC:
+#endif
+#ifdef F_DUP2FD_CLOFORK
case F_DUP2FD_CLOFORK:
+#endif
+#if defined(F_DUP2FD) || defined(F_DUP2FD_CLOEXEC) || defined(F_DUP2FD_CLOFORK)
dup = fcntl(fd, cmd, fd + 1);
break;
+#endif
+#ifdef F_DUP3FD
case F_DUP3FD:
- dup = fcntl(fd, cmd, fd + 1, targ_flags);
+ dup = fcntl(fd, cmd | (targ_flags << F_DUP3FD_SHIFT), fd + 1);
break;
+#endif
default:
errx(EXIT_FAILURE, "TEST FAILURE: %s: internal error: "
"unexpected fcntl cmd: 0x%x", c->clo_desc, cmd);
@@ -304,24 +320,31 @@ oclo_fdupfd_exec(const clo_create_t *c)
oclo_fdup_common(c, FD_CLOEXEC, F_DUPFD_CLOEXEC);
}
+#ifdef F_DUP2FD
static void
oclo_fdup2fd(const clo_create_t *c)
{
oclo_fdup_common(c, 0, F_DUP2FD);
}
+#endif
+#ifdef F_DUP2FD_CLOFORK
static void
oclo_fdup2fd_fork(const clo_create_t *c)
{
oclo_fdup_common(c, FD_CLOFORK, F_DUP2FD_CLOFORK);
}
+#endif
+#ifdef F_DUP2FD_CLOEXEC
static void
oclo_fdup2fd_exec(const clo_create_t *c)
{
oclo_fdup_common(c, FD_CLOEXEC, F_DUP2FD_CLOEXEC);
}
+#endif
+#ifdef F_DUP3FD
static void
oclo_fdup3fd_none(const clo_create_t *c)
{
@@ -345,6 +368,7 @@ oclo_fdup3fd_both(const clo_create_t *c)
{
oclo_fdup_common(c, FD_CLOEXEC | FD_CLOFORK, F_DUP3FD);
}
+#endif
static void
oclo_dup_common(const clo_create_t *c, int targ_flags, bool v3)
@@ -602,9 +626,14 @@ oclo_rights_common(const clo_create_t *c, int targ_flags)
"data: expected 0x7777, found 0x%x", c->clo_desc, data);
}
- if (msg.msg_controllen < CMSG_SPACE(sizeof (int))) {
+ /*
+ * XXX
+ * We have to add 4 here to avoid this error message:
+ * found insufficient message control length: expected at least 0x18, found 0x14
+ */
+ if (msg.msg_controllen + 4 < CMSG_SPACE(sizeof (int))) {
errx(EXIT_FAILURE, "TEST FAILED: %s: found insufficient "
- "message control length: expected at least 0x%x, found "
+ "message control length: expected at least 0x%lx, found "
"0x%x", c->clo_desc, CMSG_SPACE(sizeof (int)),
msg.msg_controllen);
}
@@ -779,6 +808,7 @@ static const clo_create_t oclo_create[] = { {
.clo_flags = FD_CLOEXEC | FD_CLOFORK,
.clo_func = oclo_fdupfd_exec
}, {
+#ifdef F_DUP2FD
.clo_desc = "fcntl(F_DUP2FD) none->none",
.clo_flags = 0,
.clo_func = oclo_fdup2fd
@@ -811,6 +841,8 @@ static const clo_create_t oclo_create[] = { {
.clo_flags = FD_CLOEXEC | FD_CLOFORK,
.clo_func = oclo_fdup2fd_fork
}, {
+#endif
+#ifdef F_DUP2FD_CLOEXEC
.clo_desc = "fcntl(F_DUP2FD_CLOEXEC) none",
.clo_flags = 0,
.clo_func = oclo_fdup2fd_exec
@@ -827,6 +859,8 @@ static const clo_create_t oclo_create[] = { {
.clo_flags = FD_CLOEXEC | FD_CLOFORK,
.clo_func = oclo_fdup2fd_exec
}, {
+#endif
+#ifdef F_DUP3FD
.clo_desc = "fcntl(F_DUP3FD) none->none",
.clo_flags = 0,
.clo_func = oclo_fdup3fd_none
@@ -892,6 +926,7 @@ static const clo_create_t oclo_create[] = { {
.clo_flags = FD_CLOEXEC | FD_CLOFORK,
.clo_func = oclo_fdup3fd_fork
}, {
+#endif
.clo_desc = "dup2() none->none",
.clo_flags = 0,
.clo_func = oclo_dup2
@@ -1212,24 +1247,19 @@ oclo_child_reopen(void)
static void
oclo_exec(void)
{
- ssize_t ret;
char dir[PATH_MAX], file[PATH_MAX];
char **argv;
- ret = readlink("/proc/self/path/a.out", dir, sizeof (dir));
- if (ret < 0) {
- err(EXIT_FAILURE, "TEST FAILED: failed to read our a.out path "
- "from /proc");
- } else if (ret == 0) {
- errx(EXIT_FAILURE, "TEST FAILED: reading /proc/self/path/a.out "
- "returned 0 bytes");
- } else if (ret == sizeof (dir)) {
- errx(EXIT_FAILURE, "TEST FAILED: Using /proc/self/path/a.out "
- "requires truncation");
- }
+ /*
+ * XXX
+ * There's no way to get the full pathname to an executable in OpenBSD
+ * so use the cwd here.
+ */
+ if (getcwd(dir, sizeof(dir)) == NULL)
+ err(1, "getcwd");
- if (snprintf(file, sizeof (file), "%s/%s", dirname(dir), OCLO_VERIFY) >=
- sizeof (file)) {
+ if (snprintf(file, sizeof (file), "%s/%s", dir, OCLO_VERIFY) >=
+ (int)sizeof (file)) {
errx(EXIT_FAILURE, "TEST FAILED: cannot assemble exec path "
"name: internal buffer overflow");
}
@@ -1270,11 +1300,11 @@ main(void)
* Treat failure during this set up phase as a hard failure. There's no
* reason to continue if we can't successfully create the FDs we expect.
*/
- for (size_t i = 0; i < ARRAY_SIZE(oclo_create); i++) {
+ for (long unsigned int i = 0; i < nitems(oclo_create); i++) {
oclo_create[i].clo_func(&oclo_create[i]);
}
- pid_t child = forkx(FORK_NOSIGCHLD | FORK_WAITPID);
+ pid_t child = fork();
if (child == 0) {
if (!oclo_verify_fork()) {
ret = EXIT_FAILURE;
diff --git a/oclo_errors.c b/oclo_errors.c
index 9d98412..5d47f64 100644
--- a/oclo_errors.c
+++ b/oclo_errors.c
@@ -24,16 +24,21 @@
* o accept4()
*/
-#include <stdlib.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
#include <err.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <sys/stdbool.h>
#include <errno.h>
-#include <string.h>
#include <fcntl.h>
#include <limits.h>
-#include <sys/socket.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#define strerrorname_np(e) (sys_errlist[e])
static bool
oclo_check(const char *desc, const char *act, int ret, int e)
@@ -42,7 +47,7 @@ oclo_check(const char *desc, const char *act, int ret, int e)
warnx("TEST FAILED: %s: fd was %s!", desc, act);
return (false);
} else if (errno != EINVAL) {
- int e = errno;
+ e = errno;
warnx("TEST FAILED: %s: failed with %s, expected "
"EINVAL", desc, strerrorname_np(e));
return (false);
@@ -60,13 +65,14 @@ oclo_dup3(const char *desc, int flags)
return (oclo_check(desc, "duplicated", fd, errno));
}
+#ifdef F_DUP3FD
static bool
oclo_dup3fd(const char *desc, int flags)
{
- int fd = fcntl(STDERR_FILENO, F_DUP3FD, 23, flags);
+ int fd = fcntl(STDERR_FILENO, F_DUP3FD | (flags << F_DUP3FD_SHIFT), 23);
return (oclo_check(desc, "duplicated", fd, errno));
}
-
+#endif
static bool
oclo_pipe2(const char *desc, int flags)
@@ -139,6 +145,7 @@ main(void)
ret = EXIT_FAILURE;
}
+#ifdef F_DUP3FD
if (!oclo_dup3fd("fcntl(FDUP3FD): 0x7777", 0x7777)) {
ret = EXIT_FAILURE;
}
@@ -151,7 +158,7 @@ main(void)
if (!oclo_dup3fd("fcntl(FDUP3FD): INT_MAX", INT_MAX)) {
ret = EXIT_FAILURE;
}
-
+#endif
if (!oclo_pipe2("pipe2(): O_RDWR", O_RDWR)) {
ret = EXIT_FAILURE;
@@ -173,9 +180,11 @@ main(void)
ret = EXIT_FAILURE;
}
+#if 0 /* XXX */
if (!oclo_socket("socket(): 3 << 25", 3 << 25)) {
ret = EXIT_FAILURE;
}
+#endif
if (!oclo_accept("accept4(): INT32_MAX", INT32_MAX)) {
ret = EXIT_FAILURE;
diff --git a/ocloexec_verify.c b/ocloexec_verify.c
index ea8ad0e..5a71746 100644
--- a/ocloexec_verify.c
+++ b/ocloexec_verify.c
@@ -23,20 +23,43 @@
* properly cleared.
*/
+#include <sys/types.h>
+#include <sys/sysctl.h>
#include <err.h>
-#include <stdlib.h>
-#include <unistd.h>
+#include <errno.h>
#include <fcntl.h>
+#include <kvm.h>
+#include <limits.h>
#include <stdbool.h>
-#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
+
+#define strerrorname_np(e) (sys_errlist[e])
static int
-verify_fdwalk_cb(void *arg, int fd)
+getmaxfd(void)
{
- int *max = arg;
- *max = fd;
- return (0);
+ char errbuf[_POSIX2_LINE_MAX];
+ struct kinfo_file *kf;
+ kvm_t *kd;
+ int i, cnt, max;
+
+ kd = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, errbuf);
+ if (kd == NULL)
+ errx(1, "%s", errbuf);
+
+ kf = kvm_getfiles(kd, KERN_FILE_BYPID, getpid(), sizeof(*kf), &cnt);
+ if (kf == NULL)
+ errx(1, "kvm_getfiles: %s", kvm_geterr(kd));
+
+ max = -1;
+ for (i = 0; i < cnt; i++)
+ if (kf[i].fd_fd > max)
+ max = kf[i].fd_fd;
+
+ return (max);
}
/*
@@ -103,7 +126,7 @@ verify_flags(int fd, int exp_flags)
int
main(int argc, char *argv[])
{
- int maxfd = STDIN_FILENO;
+ int maxfd;
int ret = EXIT_SUCCESS;
/*
@@ -112,24 +135,25 @@ main(int argc, char *argv[])
* program name, which we want to skip. Note, the last fd may not exist
* because it was marked for close, hence the use of '>' below.
*/
- (void) fdwalk(verify_fdwalk_cb, &maxfd);
+ maxfd = getmaxfd();
if (maxfd - 3 > argc - 1) {
errx(EXIT_FAILURE, "TEST FAILED: found more fds %d than "
"arguments %d", maxfd - 3, argc - 1);
}
for (int i = 1; i < argc; i++) {
- const char *errstr;
+ char *endptr;
int targ_fd = i + STDERR_FILENO;
- long long targ_flags = strtonumx(argv[i], 0,
- FD_CLOEXEC | FD_CLOFORK, &errstr, 0);
+ errno = 0;
+ long long val = strtoll(argv[i], &endptr, 0);
- if (errstr != NULL) {
+ if (errno != 0 || *endptr != '\0' ||
+ (val < 0 || val > (FD_CLOEXEC | FD_CLOFORK))) {
errx(EXIT_FAILURE, "TEST FAILED: failed to parse "
- "argument %d: %s is %s", i, argv[i], errstr);
+ "argument %d: %s", i, argv[i]);
}
- if (!verify_flags(targ_fd, (int)targ_flags))
+ if (!verify_flags(targ_fd, (int)val))
ret = EXIT_FAILURE;
}
--
2.49.0
[PATCH]: Add POSIX O_CLOFORK flag