Cleanup of err(1, "unveil") pattern: xenocara

[Ashton Fagg]

Ashton Fagg  writes:

> Hi all,
>
> I saw a discussion on here a while ago about the use of patterns like:
>
> if (unveil(some_path, "r") == -1)
>err(1, "unveil");
>
> And why that's maybe not preferable for debugging and troubleshooting
> purposes for programs which have multiple unveil calls (which happens
> fairly often).
>
> Original message here: https://marc.info/?l=openbsd-tech&m=161470144611031&w=2

Patch attached which cleans up the single occurrence of this in xenocara.

Thanks.

diff --git a/xserver/os/privsep.c b/xserver/os/privsep.c
index bbe9222c8..baba33e03 100644
--- a/xserver/os/privsep.c
+++ b/xserver/os/privsep.c
@@ -287,7 +287,7 @@ priv_init(uid_t uid, gid_t gid)
 
 	for (dev = allowed_devices; dev->name != NULL; dev++) {
 		if (unveil(dev->name, "rw") == -1 && errno != ENOENT)
-			err(1, "unveil");
+			err(1, "unveil %s", dev->name);
 	}
 	if (pledge("stdio rpath wpath sendfd proc", NULL) == -1)
 		err(1, "pledge");

Re: Cleanup of err(1, "unveil") pattern: bin, games, sbin

[Ashton Fagg]

"Theo de Raadt"  writes:

> Showing the symbolic name is not doing anywhere else in the tree.
>
> Most likely they should be
>
>  err(1, "unveil: %s", path);

Per Theo's advice, updated diffs are attached.

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 993c829f2d2..ba88d9f5f67 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -2334,11 +2334,11 @@ fork_privchld(struct interface_info *ifi, int fd, int fd2)
 		fatal("socket(AF_ROUTE, SOCK_RAW)");
 
 	if (unveil(_PATH_RESCONF, "wc") == -1)
-		fatal("unveil");
+		fatal("unveil %s", _PATH_RESCONF);
 	if (unveil("/etc/resolv.conf.tail", "r") == -1)
-		fatal("unveil");
+		fatal("unveil /etc/resolve.conf.tail");
 	if (unveil(NULL, NULL) == -1)
-		fatal("unveil");
+		fatal("unveil(NULL,NULL)");
 
 	while (quit == 0) {
 		pfd[0].fd = priv_ibuf->fd;
diff --git a/sbin/fsck_ffs/setup.c b/sbin/fsck_ffs/setup.c
index e9c922d7c37..e3706d7b759 100644
--- a/sbin/fsck_ffs/setup.c
+++ b/sbin/fsck_ffs/setup.c
@@ -105,7 +105,7 @@ setup(char *dev, int isfsdb)
 
 		if (isfsdb || !hotroot()) {
 			if (unveil("/dev", "rw") == -1)
-err(1, "unveil");
+err(1, "unveil /dev");
 			if (pledge("stdio rpath wpath getpw tty disklabel",
 			NULL) == -1)
 err(1, "pledge");
diff --git a/sbin/fsck_msdos/check.c b/sbin/fsck_msdos/check.c
index 4a2f07f1131..b011cd7dca6 100644
--- a/sbin/fsck_msdos/check.c
+++ b/sbin/fsck_msdos/check.c
@@ -55,7 +55,7 @@ checkfilesys(const char *fname)
 	int mod = 0;
 
 	if (unveil("/dev", "rw") == -1)
-		err(1, "unveil");
+		err(1, "unveil /dev");
 
 	rdonly = alwaysno;
 
diff --git a/sbin/fsck/fsck.c b/sbin/fsck/fsck.c
index 09475f346d3..ce3826885c4 100644
--- a/sbin/fsck/fsck.c
+++ b/sbin/fsck/fsck.c
@@ -110,11 +110,11 @@ main(int argc, char *argv[])
 	checkroot();
 
 	if (unveil("/dev", "rw") == -1)
-		err(1, "unveil");
+		err(1, "unveil /dev");
 	if (unveil(_PATH_FSTAB, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_FSTAB);
 	if (unveil("/sbin", "x") == -1)
-		err(1, "unveil");
+		err(1, "unveil /sbin");
 	if (pledge("stdio rpath wpath disklabel proc exec", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/sbin/nologin/nologin.c b/sbin/nologin/nologin.c
index 88bdd5f6fd7..7eb39266c56 100644
--- a/sbin/nologin/nologin.c
+++ b/sbin/nologin/nologin.c
@@ -47,7 +47,7 @@ main(int argc, char *argv[])
 	char nbuf[BUFSIZ];
 
 	if (unveil(_PATH_NOLOGIN_TXT, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_NOLOGIN_TXT);
 	if (pledge("stdio rpath", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/sbin/pflogd/privsep.c b/sbin/pflogd/privsep.c
index a1c109005cf..fa0ec37ae6a 100644
--- a/sbin/pflogd/privsep.c
+++ b/sbin/pflogd/privsep.c
@@ -134,17 +134,17 @@ priv_init(int Pflag, int argc, char *argv[])
 	setproctitle("[priv]");
 
 	if (unveil(_PATH_RESCONF, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_RESCONF);
 	if (unveil(_PATH_HOSTS, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_HOSTS);
 	if (unveil(_PATH_SERVICES, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_SERVICES);
 	if (unveil("/dev/bpf", "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil /dev/bpf");
 	if (unveil(filename, "rwc") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s", filename);
 	if (unveil(NULL, NULL) == -1)
-		err(1, "unveil");
+		err(1, "unveil(NULL,NULL)");
 
 #if 0
 	/* This needs to do bpf ioctl */
diff --git a/sbin/ping/ping.c b/sbin/ping/ping.c
index f7c3c101b25..63585019da2 100644
--- a/sbin/ping/ping.c
+++ b/sbin/ping/ping.c
@@ -266,9 +266,9 @@ main(int argc, char *argv[])
 
 	/* Cannot pledge due to special setsockopt()s below */
 	if (unveil("/", "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil /");
 	if (unveil(NULL, NULL) == -1)
-		err(1, "unveil");
+		err(1, "unveil(NULL,NULL)");
 
 	if (strcmp("ping6", __progname) == 0) {
 		v6flag = 1;
diff --git a/bin/ps/ps.c b/bin/ps/ps.c
index 84be7afe802..3b6593a46a1 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@@ -276,18 +276,18 @@ main(int argc, char *argv[])
 		errx(1, "%s", errbuf);
 
 	if (unveil(_PATH_DEVDB, "r") == -1 && errno != ENOENT)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_DEVDB);
 	if (unveil(_PATH_DEV, "r") == -1 && errno != ENOENT)
-		err(1, "unveil");
+		err(1, "unveil %s", _PATH_DEV);
 	if (swapf)
 		if (unveil(swapf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil %s", swapf);
 	if (nlistf)
 		if (unveil(nlistf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil %s", nlistf);
 	if (memf)
 		if (unveil(memf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil %s", memf);
 	if (pledge("stdio rpath getpw ps", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/sbin/shutdown/shutdown.c b/sbin/shutdown/shutdown.c
index d28eb676172..94c323c25dc 100644
--- a/sbin/shutdown/shutdown.c
+++ b/sbin/shutdown/shutdown.c
@@ -166,24 +166,24 @@ main(int argc, char *argv[])
 	}
 
 	if (unveil(_PATH_CONSOLE, "rw") == -1)
-		err(1, "unveil");
+		err(1, "unveil %s" _PATH_CONSOLE);
 	if (unveil(

Re: Cleanup of err(1, "unveil") pattern: bin, games, sbin

[Ashton Fagg]

Thanks for the review - wasn't sure if that was useful. Will
regenerate without the symbolic names.

On Sun, 2 May 2021 at 20:37, Theo de Raadt  wrote:
>
> Showing the symbolic name is not doing anywhere else in the tree.
>
> Most likely they should be
>
>err(1, "unveil: %s", path);
>
>
> Ashton Fagg  wrote:
>
> > Ashton Fagg  writes:
> >
> > > Hi all,
> > >
> > > I saw a discussion on here a while ago about the use of patterns like:
> > >
> > > if (unveil(some_path, "r") == -1)
> > >err(1, "unveil");
> > >
> > > And why that's maybe not preferable for debugging and troubleshooting
> > > purposes for programs which have multiple unveil calls (which happens
> > > fairly often).
> > >
> > > Original message here: 
> > > https://marc.info/?l=openbsd-tech&m=161470144611031&w=2
> >
> > I decided just to go ahead and do this since I noticed there's some
> > newer stuff not following this pattern (dhcpleased for example). Here are 
> > patches for:
> >
> > games/tetris
> > bin/ps
> > sbin/dhclient
> > sbin/fsck
> > sbin/fsck_msdos
> > sbin/fsck_ffs
> > sbin/nologin
> > sbin/pflogd
> > sbin/ping
> > sbin/shutdown
> > sbin/sysctl
> > sbin/unwind
> >
> > I'll tackle /usr/bin and /usr/sbin another time.
> >
> > diff --git a/bin/ps/ps.c b/bin/ps/ps.c
> > index 84be7afe802..3b6593a46a1 100644
> > --- a/bin/ps/ps.c
> > +++ b/bin/ps/ps.c
> > @@ -276,18 +276,18 @@ main(int argc, char *argv[])
> >   errx(1, "%s", errbuf);
> >
> >   if (unveil(_PATH_DEVDB, "r") == -1 && errno != ENOENT)
> > - err(1, "unveil");
> > + err(1, "unveil: _PATH_DEVDB -> %s", _PATH_DEVDB);
> >   if (unveil(_PATH_DEV, "r") == -1 && errno != ENOENT)
> > - err(1, "unveil");
> > + err(1, "unveil: _PATH_DEV -> %s", _PATH_DEV);
> >   if (swapf)
> >   if (unveil(swapf, "r") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: swapf -> %s", swapf);
> >   if (nlistf)
> >   if (unveil(nlistf, "r") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: nlistf -> %s", nlistf);
> >   if (memf)
> >   if (unveil(memf, "r") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: memf -> %s", memf);
> >   if (pledge("stdio rpath getpw ps", NULL) == -1)
> >   err(1, "pledge");
> >
> > diff --git a/games/tetris/tetris.c b/games/tetris/tetris.c
> > index 69f4532a4ac..fdb7e7d2d40 100644
> > --- a/games/tetris/tetris.c
> > +++ b/games/tetris/tetris.c
> > @@ -234,7 +234,7 @@ main(int argc, char *argv[])
> >   scr_init();
> >
> >   if (unveil(scorepath, "rwc") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: scorepath -> %s", scorepath);
> >
> >   if (pledge("stdio rpath wpath cpath tty", NULL) == -1)
> >   err(1, "pledge");
> > diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
> > index 993c829f2d2..ba88d9f5f67 100644
> > --- a/sbin/dhclient/dhclient.c
> > +++ b/sbin/dhclient/dhclient.c
> > @@ -2334,11 +2334,11 @@ fork_privchld(struct interface_info *ifi, int fd, 
> > int fd2)
> >   fatal("socket(AF_ROUTE, SOCK_RAW)");
> >
> >   if (unveil(_PATH_RESCONF, "wc") == -1)
> > - fatal("unveil");
> > + fatal("unveil: _PATH_RESCONF -> %s", _PATH_RESCONF);
> >   if (unveil("/etc/resolv.conf.tail", "r") == -1)
> > - fatal("unveil");
> > + fatal("unveil: /etc/resolve.conf.tail");
> >   if (unveil(NULL, NULL) == -1)
> > - fatal("unveil");
> > + fatal("unveil(NULL,NULL)");
> >
> >   while (quit == 0) {
> >   pfd[0].fd = priv_ibuf->fd;
> > diff --git a/sbin/fsck/fsck.c b/sbin/fsck/fsck.c
> > index 09475f346d3..ce3826885c4 100644
> > --- a/sbin/fsck/fsck.c
> > +++ b/sbin/fsck/fsck.c
> > @@ -110,11 +110,11 @@ main(int argc, char *argv[])
> >   checkroot();
> >
> >   if (unveil("/dev", "rw") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: /dev");
> >   if (unveil(_PATH_FSTAB, "r") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil: _PATH_FSTAB -> %s", _PATH_FSTAB);
> >   if (unveil("/sbin", "x") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil /sbin");
> >   if (pledge("stdio rpath wpath disklabel proc exec", NULL) == -1)
> >   err(1, "pledge");
> >
> > diff --git a/sbin/fsck_msdos/check.c b/sbin/fsck_msdos/check.c
> > index 4a2f07f1131..b011cd7dca6 100644
> > --- a/sbin/fsck_msdos/check.c
> > +++ b/sbin/fsck_msdos/check.c
> > @@ -55,7 +55,7 @@ checkfilesys(const char *fname)
> >   int mod = 0;
> >
> >   if (unveil("/dev", "rw") == -1)
> > - err(1, "unveil");
> > + err(1, "unveil /dev");
> >
> >   rdonly = alwaysno;
> >
> > diff --git a/sbin/fsck_ffs/setup.c b/sbin/fsck_ffs/setup.c
> > index e9c922d7c37..e37

Re: Cleanup of err(1, "unveil") pattern: bin, games, sbin

[Theo de Raadt]

Showing the symbolic name is not doing anywhere else in the tree.

Most likely they should be

   err(1, "unveil: %s", path);


Ashton Fagg  wrote:

> Ashton Fagg  writes:
> 
> > Hi all,
> >
> > I saw a discussion on here a while ago about the use of patterns like:
> >
> > if (unveil(some_path, "r") == -1)
> >err(1, "unveil");
> >
> > And why that's maybe not preferable for debugging and troubleshooting
> > purposes for programs which have multiple unveil calls (which happens
> > fairly often).
> >
> > Original message here: 
> > https://marc.info/?l=openbsd-tech&m=161470144611031&w=2
> 
> I decided just to go ahead and do this since I noticed there's some
> newer stuff not following this pattern (dhcpleased for example). Here are 
> patches for:
> 
> games/tetris
> bin/ps
> sbin/dhclient
> sbin/fsck
> sbin/fsck_msdos
> sbin/fsck_ffs
> sbin/nologin
> sbin/pflogd
> sbin/ping
> sbin/shutdown
> sbin/sysctl
> sbin/unwind
> 
> I'll tackle /usr/bin and /usr/sbin another time.
> 
> diff --git a/bin/ps/ps.c b/bin/ps/ps.c
> index 84be7afe802..3b6593a46a1 100644
> --- a/bin/ps/ps.c
> +++ b/bin/ps/ps.c
> @@ -276,18 +276,18 @@ main(int argc, char *argv[])
>   errx(1, "%s", errbuf);
>  
>   if (unveil(_PATH_DEVDB, "r") == -1 && errno != ENOENT)
> - err(1, "unveil");
> + err(1, "unveil: _PATH_DEVDB -> %s", _PATH_DEVDB);
>   if (unveil(_PATH_DEV, "r") == -1 && errno != ENOENT)
> - err(1, "unveil");
> + err(1, "unveil: _PATH_DEV -> %s", _PATH_DEV);
>   if (swapf)
>   if (unveil(swapf, "r") == -1)
> - err(1, "unveil");
> + err(1, "unveil: swapf -> %s", swapf);
>   if (nlistf)
>   if (unveil(nlistf, "r") == -1)
> - err(1, "unveil");
> + err(1, "unveil: nlistf -> %s", nlistf);
>   if (memf)
>   if (unveil(memf, "r") == -1)
> - err(1, "unveil");
> + err(1, "unveil: memf -> %s", memf);
>   if (pledge("stdio rpath getpw ps", NULL) == -1)
>   err(1, "pledge");
>  
> diff --git a/games/tetris/tetris.c b/games/tetris/tetris.c
> index 69f4532a4ac..fdb7e7d2d40 100644
> --- a/games/tetris/tetris.c
> +++ b/games/tetris/tetris.c
> @@ -234,7 +234,7 @@ main(int argc, char *argv[])
>   scr_init();
>  
>   if (unveil(scorepath, "rwc") == -1)
> - err(1, "unveil");
> + err(1, "unveil: scorepath -> %s", scorepath);
>  
>   if (pledge("stdio rpath wpath cpath tty", NULL) == -1)
>   err(1, "pledge");
> diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
> index 993c829f2d2..ba88d9f5f67 100644
> --- a/sbin/dhclient/dhclient.c
> +++ b/sbin/dhclient/dhclient.c
> @@ -2334,11 +2334,11 @@ fork_privchld(struct interface_info *ifi, int fd, int 
> fd2)
>   fatal("socket(AF_ROUTE, SOCK_RAW)");
>  
>   if (unveil(_PATH_RESCONF, "wc") == -1)
> - fatal("unveil");
> + fatal("unveil: _PATH_RESCONF -> %s", _PATH_RESCONF);
>   if (unveil("/etc/resolv.conf.tail", "r") == -1)
> - fatal("unveil");
> + fatal("unveil: /etc/resolve.conf.tail");
>   if (unveil(NULL, NULL) == -1)
> - fatal("unveil");
> + fatal("unveil(NULL,NULL)");
>  
>   while (quit == 0) {
>   pfd[0].fd = priv_ibuf->fd;
> diff --git a/sbin/fsck/fsck.c b/sbin/fsck/fsck.c
> index 09475f346d3..ce3826885c4 100644
> --- a/sbin/fsck/fsck.c
> +++ b/sbin/fsck/fsck.c
> @@ -110,11 +110,11 @@ main(int argc, char *argv[])
>   checkroot();
>  
>   if (unveil("/dev", "rw") == -1)
> - err(1, "unveil");
> + err(1, "unveil: /dev");
>   if (unveil(_PATH_FSTAB, "r") == -1)
> - err(1, "unveil");
> + err(1, "unveil: _PATH_FSTAB -> %s", _PATH_FSTAB);
>   if (unveil("/sbin", "x") == -1)
> - err(1, "unveil");
> + err(1, "unveil /sbin");
>   if (pledge("stdio rpath wpath disklabel proc exec", NULL) == -1)
>   err(1, "pledge");
>  
> diff --git a/sbin/fsck_msdos/check.c b/sbin/fsck_msdos/check.c
> index 4a2f07f1131..b011cd7dca6 100644
> --- a/sbin/fsck_msdos/check.c
> +++ b/sbin/fsck_msdos/check.c
> @@ -55,7 +55,7 @@ checkfilesys(const char *fname)
>   int mod = 0;
>  
>   if (unveil("/dev", "rw") == -1)
> - err(1, "unveil");
> + err(1, "unveil /dev");
>  
>   rdonly = alwaysno;
>  
> diff --git a/sbin/fsck_ffs/setup.c b/sbin/fsck_ffs/setup.c
> index e9c922d7c37..e3706d7b759 100644
> --- a/sbin/fsck_ffs/setup.c
> +++ b/sbin/fsck_ffs/setup.c
> @@ -105,7 +105,7 @@ setup(char *dev, int isfsdb)
>  
>   if (isfsdb || !hotroot()) {
>   if (unveil("/dev", "rw") == -1)
> - err(1, "unveil");
> + err(1, "unveil /dev");
>   if (pledge("stdio rpath wpath getpw tty 

Cleanup of err(1, "unveil") pattern: bin, games, sbin

[Ashton Fagg]

Ashton Fagg  writes:

> Hi all,
>
> I saw a discussion on here a while ago about the use of patterns like:
>
> if (unveil(some_path, "r") == -1)
>err(1, "unveil");
>
> And why that's maybe not preferable for debugging and troubleshooting
> purposes for programs which have multiple unveil calls (which happens
> fairly often).
>
> Original message here: https://marc.info/?l=openbsd-tech&m=161470144611031&w=2

I decided just to go ahead and do this since I noticed there's some
newer stuff not following this pattern (dhcpleased for example). Here are 
patches for:

games/tetris
bin/ps
sbin/dhclient
sbin/fsck
sbin/fsck_msdos
sbin/fsck_ffs
sbin/nologin
sbin/pflogd
sbin/ping
sbin/shutdown
sbin/sysctl
sbin/unwind

I'll tackle /usr/bin and /usr/sbin another time.

diff --git a/bin/ps/ps.c b/bin/ps/ps.c
index 84be7afe802..3b6593a46a1 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@@ -276,18 +276,18 @@ main(int argc, char *argv[])
 		errx(1, "%s", errbuf);
 
 	if (unveil(_PATH_DEVDB, "r") == -1 && errno != ENOENT)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_DEVDB -> %s", _PATH_DEVDB);
 	if (unveil(_PATH_DEV, "r") == -1 && errno != ENOENT)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_DEV -> %s", _PATH_DEV);
 	if (swapf)
 		if (unveil(swapf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil: swapf -> %s", swapf);
 	if (nlistf)
 		if (unveil(nlistf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil: nlistf -> %s", nlistf);
 	if (memf)
 		if (unveil(memf, "r") == -1)
-			err(1, "unveil");
+			err(1, "unveil: memf -> %s", memf);
 	if (pledge("stdio rpath getpw ps", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/games/tetris/tetris.c b/games/tetris/tetris.c
index 69f4532a4ac..fdb7e7d2d40 100644
--- a/games/tetris/tetris.c
+++ b/games/tetris/tetris.c
@@ -234,7 +234,7 @@ main(int argc, char *argv[])
 	scr_init();
 
 	if (unveil(scorepath, "rwc") == -1)
-		err(1, "unveil");
+		err(1, "unveil: scorepath -> %s", scorepath);
 
 	if (pledge("stdio rpath wpath cpath tty", NULL) == -1)
 		err(1, "pledge");
diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 993c829f2d2..ba88d9f5f67 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -2334,11 +2334,11 @@ fork_privchld(struct interface_info *ifi, int fd, int fd2)
 		fatal("socket(AF_ROUTE, SOCK_RAW)");
 
 	if (unveil(_PATH_RESCONF, "wc") == -1)
-		fatal("unveil");
+		fatal("unveil: _PATH_RESCONF -> %s", _PATH_RESCONF);
 	if (unveil("/etc/resolv.conf.tail", "r") == -1)
-		fatal("unveil");
+		fatal("unveil: /etc/resolve.conf.tail");
 	if (unveil(NULL, NULL) == -1)
-		fatal("unveil");
+		fatal("unveil(NULL,NULL)");
 
 	while (quit == 0) {
 		pfd[0].fd = priv_ibuf->fd;
diff --git a/sbin/fsck/fsck.c b/sbin/fsck/fsck.c
index 09475f346d3..ce3826885c4 100644
--- a/sbin/fsck/fsck.c
+++ b/sbin/fsck/fsck.c
@@ -110,11 +110,11 @@ main(int argc, char *argv[])
 	checkroot();
 
 	if (unveil("/dev", "rw") == -1)
-		err(1, "unveil");
+		err(1, "unveil: /dev");
 	if (unveil(_PATH_FSTAB, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_FSTAB -> %s", _PATH_FSTAB);
 	if (unveil("/sbin", "x") == -1)
-		err(1, "unveil");
+		err(1, "unveil /sbin");
 	if (pledge("stdio rpath wpath disklabel proc exec", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/sbin/fsck_msdos/check.c b/sbin/fsck_msdos/check.c
index 4a2f07f1131..b011cd7dca6 100644
--- a/sbin/fsck_msdos/check.c
+++ b/sbin/fsck_msdos/check.c
@@ -55,7 +55,7 @@ checkfilesys(const char *fname)
 	int mod = 0;
 
 	if (unveil("/dev", "rw") == -1)
-		err(1, "unveil");
+		err(1, "unveil /dev");
 
 	rdonly = alwaysno;
 
diff --git a/sbin/fsck_ffs/setup.c b/sbin/fsck_ffs/setup.c
index e9c922d7c37..e3706d7b759 100644
--- a/sbin/fsck_ffs/setup.c
+++ b/sbin/fsck_ffs/setup.c
@@ -105,7 +105,7 @@ setup(char *dev, int isfsdb)
 
 		if (isfsdb || !hotroot()) {
 			if (unveil("/dev", "rw") == -1)
-err(1, "unveil");
+err(1, "unveil /dev");
 			if (pledge("stdio rpath wpath getpw tty disklabel",
 			NULL) == -1)
 err(1, "pledge");
diff --git a/sbin/nologin/nologin.c b/sbin/nologin/nologin.c
index 88bdd5f6fd7..7eb39266c56 100644
--- a/sbin/nologin/nologin.c
+++ b/sbin/nologin/nologin.c
@@ -47,7 +47,7 @@ main(int argc, char *argv[])
 	char nbuf[BUFSIZ];
 
 	if (unveil(_PATH_NOLOGIN_TXT, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_NOLOGIN_TXT -> %s", _PATH_NOLOGIN_TXT);
 	if (pledge("stdio rpath", NULL) == -1)
 		err(1, "pledge");
 
diff --git a/sbin/pflogd/privsep.c b/sbin/pflogd/privsep.c
index a1c109005cf..fa0ec37ae6a 100644
--- a/sbin/pflogd/privsep.c
+++ b/sbin/pflogd/privsep.c
@@ -134,17 +134,17 @@ priv_init(int Pflag, int argc, char *argv[])
 	setproctitle("[priv]");
 
 	if (unveil(_PATH_RESCONF, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_RECONF -> %s", _PATH_RESCONF);
 	if (unveil(_PATH_HOSTS, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_HOSTS -> %s", _PATH_HOSTS);
 	if (unveil(_PATH_SERVICES, "r") == -1)
-		err(1, "unveil");
+		err(1, "unveil: _PATH_SERVIC

Fix compiler warning from sysctl.c

[Ashton Fagg]

Fixes the following warning:

sysctl.c:835:18: warning: format specifies type 'char *' but the argument has 
type 'void *' [-Wformat]
string, newval);
^~
sysctl.c:840:18: warning: format specifies type 'char *' but the argument has 
type 'void *' [-Wformat]
string, newval);

Appears as though this was fixed in lines above these two instances, but
these were still not properly cast.

diff --git a/sbin/sysctl/sysctl.c b/sbin/sysctl/sysctl.c
index 5e9e562d308..a03220d7d56 100644
--- a/sbin/sysctl/sysctl.c
+++ b/sbin/sysctl/sysctl.c
@@ -832,12 +832,12 @@ parse(char *string, int flags)
 newval);
 if (len == -1) {
 	warnx("%s: hex string %s: invalid",
-	string, newval);
+	string, (char *)newval);
 	return;
 }
 if (len > sizeof(hex)) {
 	warnx("%s: hex string %s: too long",
-	string, newval);
+	string, (char *)newval);
 	return;
 }
 

Diff for www:OpenBSD/powerpc64

[bsd]

Hi,

Here a diff for www page: OpenBSD/powerpc64

Hi. According to page 68, 6.8 was the first version for the platform.

Right?


Index: powerpc64.html
===
RCS file: /cvs/www/powerpc64.html,v
retrieving revision 1.7
diff -u -r1.7 powerpc64.html
--- powerpc64.html  30 Apr 2021 16:14:12 -  1.7
+++ powerpc64.html  2 May 2021 18:10:32 -
@@ -39,7 +39,7 @@
 History:
 
 
-OpenBSD 6.9 was the first official release of OpenBSD/powerpc64.
+OpenBSD 6.8 was the first official release of OpenBSD/powerpc64.
 
 
 And since there are always little bits missing, work is continuing...

Re: diff for wireguard examples on faq17.html

[Jim Hoffman]

I'll work a diff for wg(4).

Thanks,
Jim

On Sun, May 02, 2021 at 09:57:33AM -0600, Theo de Raadt wrote:
> If the manual page is weak, it should be improved.
> 
> The FAQ cannot make up for it.  The FAQ requires online interaction
> and far fewer people read it.
> 
> 

Re: diff for wireguard examples on faq17.html

[Jim Hoffman]

Sorry accidently replied directly to Theo instead of the list.

Yes to both of your questions. Sorry if this is noise or spam. 

Section of manpage:
EXAMPLES
 Create two wg interfaces in separate rdomain(4)s, which is of no
 practical use but demonstrates two interfaces on the same machine:

On Sun, May 02, 2021 at 09:25:21AM -0600, Theo de Raadt wrote:
> Isn't a majority of this already in the manual page?
> 
> Or alternatively, does your attempt to create a FAQ entry indicate that
> the manual page is weak or unsuitable?
> 
> 
> 

Diff for www:FAQ ports/ports

[bsd]

Hi,

Here a diff for www page: FAQ ports/ports

Hi, see this typo error on the page.

Right?


Index: faq/ports/ports.html
===
RCS file: /cvs/www/faq/ports/ports.html,v
retrieving revision 1.57
diff -u -r1.57 ports.html
--- faq/ports/ports.html4 Dec 2020 17:08:16 -   1.57
+++ faq/ports/ports.html2 May 2021 13:42:42 -
@@ -759,7 +759,7 @@
 available (for example, if the main port is neomutt-20201127,
 the debug package will be debug-neomutt-20201127).
 These contain debug symbols which have been separated into a different
-ile; GDB knows how to load it automatically.
+file; GDB knows how to load it automatically.
 The debug package must match the main package.
 If you are using snapshots, you may need to reinstall to ensure that
 they are from the same build.

diff for wireguard examples on faq17.html

[Jim Hoffman]

Hi,

There has been a lot of interest in WireGuard on the OpenBSD subreddit. So here
is a patch that provides a few basic examples for usage.

Thanks,
Jim


Index: www/faq/faq17.html
===
RCS file: /cvs/www/faq/faq17.html,v
retrieving revision 1.16
diff -u -p -u -r1.16 faq17.html
--- www/faq/faq17.html  12 Dec 2020 19:07:25 -  1.16
+++ www/faq/faq17.html  2 May 2021 15:08:42 -
@@ -54,6 +54,7 @@ FAQ - Virtual Private Networks (VPN)
 With a Windows Client
   
   Connecting to an IKEv1/L2TP OpenBSD VPN
+  WireGuard
 
 
 
@@ -78,12 +79,6 @@ also available and, coupled with
 https://man.openbsd.org/npppd";>npppd(8), it allows you to build
 an IKEv1/L2TP VPN where IKEv2 can't be deployed.
 
-
-Native WireGuard support is also available via the
-https://man.openbsd.org/wg";>wg(4) device.
-As the manual explains, it can be configured the same way as all other
-network interfaces in OpenBSD.
-
 Authentication
 
 https://man.openbsd.org/iked";>iked(8) supports the following
@@ -644,3 +639,169 @@ OpenBSD doesn't provide an L2TP client b
 
 Refer to /usr/local/share/doc/pkg-readmes/xl2tpd for instructions
 on how to properly setup the L2TP client.
+
+WireGuard
+
+
+Native WireGuard support is also available via the
+https://man.openbsd.org/wg";>wg(4) device.
+As the manual explains, it can be configured the same way as all other
+network interfaces in OpenBSD.
+
+
+WireGuard is an interface based VPN tunnel, which requires more knowledge
+about networking to get functioning.
+It can act as a responder or initiator or both where possible.
+WireGuard is a connectionless based tunnel, meaning there is no need to worry
+about daemons or state of the tunnel.
+The encryption / decryption key that is negotiated has a very short life span 
of
+2 minutes and does not rely on the last packet recieved; so it is very 
resilient
+against packet loss.
+
+
+Required to forward traffic over WireGuard interfaces.
+
+
+# sysctl net.inet.ip.forwarding=1
+
+
+Client to Server example
+
+Server configuration (responder)
+
+
+Create a persistnent private key and interface, set listening port, get the
+pubkey that the client will need.
+
+
+server# echo "wgkey `openssl rand -base64 32`" > /etc/hostname.wg0
+server# # sh /etc/netstart wg0
+server# ifconfig wg0 wgport 51820
+server# ifconfig wg0 | grep wgpubkey | cut -d ‘ ‘ -f 2
+zQfRbQPcQiLppc55LJWbFCdrnHdpxZTg47VQmJG6heE=
+server# ifconfig wg0 inet 192.0.2.1/24
+server# ifconfig wg0 wgpeer f+wtDqJrNnSIRqOCCiBa4YWGZ58WLSo5b5oWjBQt6Xg= \
+wgpsk jPNozq8SGbXk5ACrfAHEA3/O1jWlrhiCJ4ibvon3oqc= \
+wgaip 192.0.2.2/32
+
+
+
+PF rules needed for clients to establish a connection and enter and leave on 
the
+wg0 interface on the server.
+The nat-to rule is only needed if you want the client to be able to use the
+internet of the server.
+
+
+pass in on egress inet proto udp to port 51820
+match out on egress from wg0:network nat-to egress # needed for client to surf 
the internet
+pass on wg keep state (if-bound)
+
+
+Client configuration (initiator)
+
+
+Create a persistnent private key and interface and get the pubkey that the
+server will need.
+
+
+client# echo "wgkey `openssl rand -base64 32`" > /etc/hostname.wg0
+client# # sh /etc/netstart wg0
+clinet# ifconfig wg0 | grep wgpubkey | cut -d ‘ ‘ -f 2
+f+wtDqJrNnSIRqOCCiBa4YWGZ58WLSo5b5oWjBQt6Xg=
+client# ifconfig wg0 inet 192.0.2.2/24
+client# ifconfig wg0 wgpeer zQfRbQPcQiLppc55LJWbFCdrnHdpxZTg47VQmJG6heE= \
+wgpsk jPNozq8SGbXk5ACrfAHEA3/O1jWlrhiCJ4ibvon3oqc= \
+wgendpoint a.b.c.d 51820 wgaip 0.0.0.0/0
+
+
+
+PF rules needed for traffic to enter and leave on the wg0 interface on the
+client.
+
+
+pass on wg keep state (if-bound)
+
+
+
+Routes needed on client to tunnel all traffic to / through the server.
+
+
+client# route add -priority 2 a.b.c.d `netstat -rn | grep default | \
+awk ‘{print $2}’`
+client# route add -priority 7 default 192.0.2.1/24
+
+
+Server to Server example
+
+
+This example shows 2 servers each with another subnet connected to them. Pay
+close attention to the wgaip info. There can be multiple allowed IP's or
+subnets each with their own wgaip for the wgpeer line.
+
+
+Server 1 configuration (initiator and responder)
+
+
+Create a persistnent private key and interface, set listening port, get the
+pubkey that server 2 will need.
+
+
+server1# echo "wgkey `openssl rand -base64 32`" > /etc/hostname.wg0
+server1# # sh /etc/netstart wg0
+server1# ifconfig wg0 wgport 51820
+server1# ifconfig wg0 | grep wgpubkey | cut -d ‘ ‘ -f 2
+zQfRbQPcQiLppc55LJWbFCdrnHdpxZTg47VQmJG6heE=
+server1# ifconfig wg0 inet 192.0.2.1/24
+server1# ifconfig wg0 wgpeer f+wtDqJrNnSIRqOCCiBa4YWGZ58WLSo5b5oWjBQt6Xg= \
+wgpsk jPNozq8SGbXk5ACrfAHEA3/O1jWlrhiCJ4ibvon3oqc= \
+wgendpoint e.f.g.h 51820 wgaip 192.0.2.2/32 wgaip 
10.0.1.0/24
+
+
+
+PF rules needed for server 2 to establi

Re: added support for precompressed static files on httpd(so sorry about my other email that was unreadable)

[alloca]

> your diff has numerous issues.
What exactly do you mean by issues? Do you mean bugs?
> It also seems you've either purposely broken comment blocks or
> are commenting out sections of existing code.
Yes i am commenting out sections of existing code, i didn't
comment the code out at the start but no matter what i
tried i got error code 400
I completely understand if you don't wanna accept the patch
for this reason.
> See https://www.openbsd.org/faq/faq5.html#Diff as well as style(9) man
> page.
I will, should i resubmit the patch following style(9) and the
diff part of fqa? Or will it still be rejected due to commenting out existing 
code?

Re: relayd patch for websocket upgrade

[Jonathon Fletcher]

On Sun, Mar 07, 2021 at 06:22:04PM -0800, Jonathon Fletcher wrote:
> On Sun, Mar 07, 2021 at 06:46:33PM +0100, Marcus MERIGHI wrote:
> > Hello Jonathon!
> > 
> > welcome to the party:
> > 
> > https://marc.info/?t=15833439123
> > 
> > especially the two comments by sthen@:
> > 
> > https://marc.info/?m=161349608614743
> > https://marc.info/?m=16135019371
> > 
> > reyk@ removed from CC: on purpose: 
> > https://twitter.com/reykfloeter/status/1284868070901776384
> > 
> > Marcus
> > 
> > jonathon.fletc...@gmail.com (Jonathon Fletcher), 2021.03.06 (Sat) 21:02 
> > (CET):
> > > When relayd relays a connection upgrade to a websocket, it relays
> > > the outbound "Connection: Upgrade" header from the interal server.
> > > 
> > > It also tags on a "Connection: close" header to the outbound
> > > response - ie the response goes out with two "Connection"
> > > header lines.
> > > 
> > > Chrome and Netscape work despite the double upgrade/close connection 
> > > headers. Safari fails.
> > > 
> > > Small patch below against 6.8 to only send the "Connection: close"
> > > header if we are not handling a http_status 101.
> > > 
> > > Thanks,
> > > Jonathon
> > > 
> > > 
> > > cvs -q -d /cvs diff -ub -rOPENBSD_6_8 usr.sbin/relayd/relay_http.c
> > > 
> > > 

snip

> Marcus,
> 
> I did not realize that there was already a party. Apologies for my
> previous, duplicate, patch.
> 
> Reading through the thread, I came to the conclusion that the comments
> worried that the previous patch(es) were not specific enough.
> 
> The current relayd behaviour is that outbound http responses have a
> "Connection: close" header/value attached to them by relayd.
> This can result in multiple "Connection" headers in the response
> which is .. not good.
> 
> The current behaviour is because relayd does not handle repeated http
> request/response sequences after the first one and prefers to force the
> http session to close. Fortunately for websockets, the protocol after
> the websocket upgrade is not http and so there is no need for relayd
> to look for or process http headers after the upgrade.
> 
> Here is an updated patch. This avoids the incorrect current in-tree
> behaviour in the following specific sitations:
> 
> 1: The headers for an outbound (internal -> external) response already
>include "Connection: Upgrade", "Upgrade: websocket" and the config
>permits websocket upgrades, or
> 
> 2: The headers for an outbound response already include a Connection
>header with the value "close" - so do not send a duplicate as the
>in-tree code currently does.
> 
> I think this is specfic enough for now. In order for a websocket upgrade
> to work the external client has to request it and the internal server 
> has to respond in agreement.
> 
> I am explicit about websocket upgrades in my configs: I require the
> "Connection" and "Upgrade" headers in the rule that directs traffic
> to the websocket pool:
> 
> 
> pass request quick \
> header "Host" value "ws.example.com" \
> header "Connection" value "Upgrade" \
> header "Upgrade" value "websocket" \
> forward to 
> 
> 
> This is for my use cases (tls accelerator) and relayd is adept at
> handling them. I really appreciate the functionality of relayd in base.
> 
> Let me know if there are specific concerns about the patch below or
> if there are specific preferred ways to accomplish better compliance
> with the RFC within the context of relayd.
> 
> Thanks,
> Jonathon
> 
> The Connection header is covered in:
> 
> https://tools.ietf.org/html/rfc7230#section-6
> 

Here is the same relayd websocket upgrade patch as above, but
against OPENBSD_6_9.

Thanks,
Jonathon

Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.81
diff -u -p -u -b -r1.81 relay_http.c
--- usr.sbin/relayd/relay_http.c24 Mar 2021 20:59:54 -  1.81
+++ usr.sbin/relayd/relay_http.c2 May 2021 17:45:09 -
@@ -180,6 +180,8 @@ relay_read_http(struct bufferevent *bev,
struct http_method_node *hmn;
struct http_session *hs;
enum httpmethod  request_method;
+   struct kv   *connection_close = NULL;
+   int ws_response = 0;
 
getmonotime(&con->se_tv_last);
cre->timedout = 0;
@@ -493,6 +495,7 @@ relay_read_http(struct bufferevent *bev,
"Connection", "upgrade", ",");
upgrade_ws = kv_find_value(&desc->http_headers,
"Upgrade", "websocket", ",");
+   ws_response = 0;
if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
relay_abort_http(con, 403,
@@ -511,6 +514,7 @@ relay_read_http(struct bufferevent *bev,
des

Re: sdhc(4) attachment for Zynq-7000

[Mark Kettenis]

> Date: Sun, 2 May 2021 15:46:30 +
> From: Visa Hankala 
> 
> On Sun, May 02, 2021 at 05:28:24PM +0200, Mark Kettenis wrote:
> > > Date: Sun, 2 May 2021 14:21:29 +
> > > From: Visa Hankala 
> > > 
> > > Zynq-7000 has a variant of Arasan SD controller that is not recognized
> > > by the fdt glue. The diff below fixes this.
> > > 
> > > The controller's capabilities register lacks the base clock frequency.
> > > The attachment glue has to provide this parameter.
> > > 
> > > OK?
> > > 
> > > Index: dev/fdt/sdhc_fdt.c
> > > ===
> > > RCS file: src/sys/dev/fdt/sdhc_fdt.c,v
> > > retrieving revision 1.15
> > > diff -u -p -r1.15 sdhc_fdt.c
> > > --- dev/fdt/sdhc_fdt.c5 Apr 2021 09:31:45 -   1.15
> > > +++ dev/fdt/sdhc_fdt.c2 May 2021 14:07:45 -
> > > @@ -126,6 +126,7 @@ sdhc_fdt_match(struct device *parent, vo
> > >   struct fdt_attach_args *faa = aux;
> > >  
> > >   return (OF_is_compatible(faa->fa_node, "arasan,sdhci-5.1") ||
> > > + OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a") ||
> > >   OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2") ||
> > >   OF_is_compatible(faa->fa_node, "brcm,bcm2835-sdhci") ||
> > >   OF_is_compatible(faa->fa_node, "marvell,armada-3700-sdhci") ||
> > > @@ -232,6 +233,11 @@ sdhc_fdt_attach(struct device *parent, s
> > >   sc->sc.sc_flags |= SDHC_F_NODDR50;
> > >   }
> > >  
> > > + if (OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a")) {
> > > + freq = clock_get_frequency(faa->fa_node, "clk_xin");
> > > + sc->sc.sc_clkbase = (freq + 500) / 1000;
> > 
> > Is there a particular reason why you are trying to round to the
> > nearest kHz here?  We don't do that for any of the other cases where
> > we set sc_clkbase, and I'd prefer not to have any special cases that
> > aren't necessary.
> 
> In my case, the rounding makes a displayed value look more sensible.
> 
> sdhc0 at simplebus0
> sdhc0: SDHC 2.0, 100 MHz base clock
> sdmmc0 at sdhc0: 4-bit, sd high-speed, mmc high-speed, dma
> 
> The clock computations use frequency  Hz as input. This value
> is multiplied and divided, giving 9990 Hz (which already has some
> truncation). If this was truncated to MHz, the result would be 99 MHz.
> 
> However, if the rounding is not wanted, I can leave it out.

I'd prefer it that way.  ok kettenis@ with that change.

This will potentially result in a slight overclocking of the bus, but
it will be smaller than 1 kHz on a minimal clock rate of 400 kHz.
This should be well within the tolerances of the SD card
specification, so I don't expect any issues.

If we want to fix this, I'd suggest we change sc_clkbase to be in Hz
rather than kHz and maybe do some rounding when we print the value in
MHz.  That way we avoid double rounding.  But it isn't very important.

> > > + }
> > > +
> > >   if (OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2"))
> > >   sc->sc.sc_flags |= SDHC_F_NOPWR0;
> > > 
> 

Re: diff for wireguard examples on faq17.html

[Theo de Raadt]

If the manual page is weak, it should be improved.

The FAQ cannot make up for it.  The FAQ requires online interaction
and far fewer people read it.


> Sorry accidently replied directly to Theo instead of the list.
> 
> Yes to both of your questions. Sorry if this is noise or spam. 
> 
> Section of manpage:
> EXAMPLES
>  Create two wg interfaces in separate rdomain(4)s, which is of no
>  practical use but demonstrates two interfaces on the same machine:
> 
> On Sun, May 02, 2021 at 09:25:21AM -0600, Theo de Raadt wrote:
> > Isn't a majority of this already in the manual page?
> > 
> > Or alternatively, does your attempt to create a FAQ entry indicate that
> > the manual page is weak or unsuitable?
> > 
> > 
> > 
> 

Re: sdhc(4) attachment for Zynq-7000

[Visa Hankala]

On Sun, May 02, 2021 at 05:28:24PM +0200, Mark Kettenis wrote:
> > Date: Sun, 2 May 2021 14:21:29 +
> > From: Visa Hankala 
> > 
> > Zynq-7000 has a variant of Arasan SD controller that is not recognized
> > by the fdt glue. The diff below fixes this.
> > 
> > The controller's capabilities register lacks the base clock frequency.
> > The attachment glue has to provide this parameter.
> > 
> > OK?
> > 
> > Index: dev/fdt/sdhc_fdt.c
> > ===
> > RCS file: src/sys/dev/fdt/sdhc_fdt.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 sdhc_fdt.c
> > --- dev/fdt/sdhc_fdt.c  5 Apr 2021 09:31:45 -   1.15
> > +++ dev/fdt/sdhc_fdt.c  2 May 2021 14:07:45 -
> > @@ -126,6 +126,7 @@ sdhc_fdt_match(struct device *parent, vo
> > struct fdt_attach_args *faa = aux;
> >  
> > return (OF_is_compatible(faa->fa_node, "arasan,sdhci-5.1") ||
> > +   OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a") ||
> > OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2") ||
> > OF_is_compatible(faa->fa_node, "brcm,bcm2835-sdhci") ||
> > OF_is_compatible(faa->fa_node, "marvell,armada-3700-sdhci") ||
> > @@ -232,6 +233,11 @@ sdhc_fdt_attach(struct device *parent, s
> > sc->sc.sc_flags |= SDHC_F_NODDR50;
> > }
> >  
> > +   if (OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a")) {
> > +   freq = clock_get_frequency(faa->fa_node, "clk_xin");
> > +   sc->sc.sc_clkbase = (freq + 500) / 1000;
> 
> Is there a particular reason why you are trying to round to the
> nearest kHz here?  We don't do that for any of the other cases where
> we set sc_clkbase, and I'd prefer not to have any special cases that
> aren't necessary.

In my case, the rounding makes a displayed value look more sensible.

sdhc0 at simplebus0
sdhc0: SDHC 2.0, 100 MHz base clock
sdmmc0 at sdhc0: 4-bit, sd high-speed, mmc high-speed, dma

The clock computations use frequency  Hz as input. This value
is multiplied and divided, giving 9990 Hz (which already has some
truncation). If this was truncated to MHz, the result would be 99 MHz.

However, if the rounding is not wanted, I can leave it out.

> > +   }
> > +
> > if (OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2"))
> > sc->sc.sc_flags |= SDHC_F_NOPWR0;
> > 

Re: sdhc(4) attachment for Zynq-7000

[Mark Kettenis]

> Date: Sun, 2 May 2021 14:21:29 +
> From: Visa Hankala 
> 
> Zynq-7000 has a variant of Arasan SD controller that is not recognized
> by the fdt glue. The diff below fixes this.
> 
> The controller's capabilities register lacks the base clock frequency.
> The attachment glue has to provide this parameter.
> 
> OK?
> 
> Index: dev/fdt/sdhc_fdt.c
> ===
> RCS file: src/sys/dev/fdt/sdhc_fdt.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 sdhc_fdt.c
> --- dev/fdt/sdhc_fdt.c5 Apr 2021 09:31:45 -   1.15
> +++ dev/fdt/sdhc_fdt.c2 May 2021 14:07:45 -
> @@ -126,6 +126,7 @@ sdhc_fdt_match(struct device *parent, vo
>   struct fdt_attach_args *faa = aux;
>  
>   return (OF_is_compatible(faa->fa_node, "arasan,sdhci-5.1") ||
> + OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a") ||
>   OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2") ||
>   OF_is_compatible(faa->fa_node, "brcm,bcm2835-sdhci") ||
>   OF_is_compatible(faa->fa_node, "marvell,armada-3700-sdhci") ||
> @@ -232,6 +233,11 @@ sdhc_fdt_attach(struct device *parent, s
>   sc->sc.sc_flags |= SDHC_F_NODDR50;
>   }
>  
> + if (OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a")) {
> + freq = clock_get_frequency(faa->fa_node, "clk_xin");
> + sc->sc.sc_clkbase = (freq + 500) / 1000;

Is there a particular reason why you are trying to round to the
nearest kHz here?  We don't do that for any of the other cases where
we set sc_clkbase, and I'd prefer not to have any special cases that
aren't necessary.

> + }
> +
>   if (OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2"))
>   sc->sc.sc_flags |= SDHC_F_NOPWR0;
> 

Re: Diff for www:FAQ ports/ports

[Stuart Henderson]

thanks, committed.

On 2021/05/02 15:46, b...@stephane-huc.net wrote:
> Hi,
> 
> Here a diff for www page: FAQ ports/ports
> 
> Hi, see this typo error on the page.
> 
> Right?
> 
> 
> Index: faq/ports/ports.html
> ===
> RCS file: /cvs/www/faq/ports/ports.html,v
> retrieving revision 1.57
> diff -u -r1.57 ports.html
> --- faq/ports/ports.html  4 Dec 2020 17:08:16 -   1.57
> +++ faq/ports/ports.html  2 May 2021 13:42:42 -
> @@ -759,7 +759,7 @@
>  available (for example, if the main port is neomutt-20201127,
>  the debug package will be debug-neomutt-20201127).
>  These contain debug symbols which have been separated into a different
> -ile; GDB knows how to load it automatically.
> +file; GDB knows how to load it automatically.
>  The debug package must match the main package.
>  If you are using snapshots, you may need to reinstall to ensure that
>  they are from the same build.
> 

Re: diff for wireguard examples on faq17.html

[Theo de Raadt]

Isn't a majority of this already in the manual page?

Or alternatively, does your attempt to create a FAQ entry indicate that
the manual page is weak or unsuitable?



> There has been a lot of interest in WireGuard on the OpenBSD subreddit. So 
> here
> is a patch that provides a few basic examples for usage.
> 
> Thanks,
> Jim
> 
> 
> Index: www/faq/faq17.html
> ===
> RCS file: /cvs/www/faq/faq17.html,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 faq17.html
> --- www/faq/faq17.html12 Dec 2020 19:07:25 -  1.16
> +++ www/faq/faq17.html2 May 2021 15:08:42 -
> @@ -54,6 +54,7 @@ FAQ - Virtual Private Networks (VPN)
>  With a Windows Client
>
>Connecting to an IKEv1/L2TP OpenBSD VPN
> +  WireGuard
>  
>  
>  
> @@ -78,12 +79,6 @@ also available and, coupled with
>  https://man.openbsd.org/npppd";>npppd(8), it allows you to build
>  an IKEv1/L2TP VPN where IKEv2 can't be deployed.
>  
> -
> -Native WireGuard support is also available via the
> -https://man.openbsd.org/wg";>wg(4) device.
> -As the manual explains, it can be configured the same way as all other
> -network interfaces in OpenBSD.
> -
>  Authentication
>  
>  https://man.openbsd.org/iked";>iked(8) supports the following
> @@ -644,3 +639,169 @@ OpenBSD doesn't provide an L2TP client b
>  
>  Refer to /usr/local/share/doc/pkg-readmes/xl2tpd for 
> instructions
>  on how to properly setup the L2TP client.
> +
> +WireGuard
> +
> +
> +Native WireGuard support is also available via the
> +https://man.openbsd.org/wg";>wg(4) device.
> +As the manual explains, it can be configured the same way as all other
> +network interfaces in OpenBSD.
> +
> +
> +WireGuard is an interface based VPN tunnel, which requires more knowledge
> +about networking to get functioning.
> +It can act as a responder or initiator or both where possible.
> +WireGuard is a connectionless based tunnel, meaning there is no need to worry
> +about daemons or state of the tunnel.
> +The encryption / decryption key that is negotiated has a very short life 
> span of
> +2 minutes and does not rely on the last packet recieved; so it is very 
> resilient
> +against packet loss.
> +
> +
> +Required to forward traffic over WireGuard interfaces.
> +
> +
> +# sysctl net.inet.ip.forwarding=1
> +
> +
> +Client to Server example
> +
> +Server configuration (responder)
> +
> +
> +Create a persistnent private key and interface, set listening port, get the
> +pubkey that the client will need.
> +
> +
> +server# echo "wgkey `openssl rand -base64 32`" > /etc/hostname.wg0
> +server# # sh /etc/netstart wg0
> +server# ifconfig wg0 wgport 51820
> +server# ifconfig wg0 | grep wgpubkey | cut -d ‘ ‘ -f 2
> +zQfRbQPcQiLppc55LJWbFCdrnHdpxZTg47VQmJG6heE=
> +server# ifconfig wg0 inet 192.0.2.1/24
> +server# ifconfig wg0 wgpeer f+wtDqJrNnSIRqOCCiBa4YWGZ58WLSo5b5oWjBQt6Xg= \
> +wgpsk jPNozq8SGbXk5ACrfAHEA3/O1jWlrhiCJ4ibvon3oqc= \
> +wgaip 192.0.2.2/32
> +
> +
> +
> +PF rules needed for clients to establish a connection and enter and leave on 
> the
> +wg0 interface on the server.
> +The nat-to rule is only needed if you want the client to be able to use the
> +internet of the server.
> +
> +
> +pass in on egress inet proto udp to port 51820
> +match out on egress from wg0:network nat-to egress # needed for client to 
> surf the internet
> +pass on wg keep state (if-bound)
> +
> +
> +Client configuration (initiator)
> +
> +
> +Create a persistnent private key and interface and get the pubkey that the
> +server will need.
> +
> +
> +client# echo "wgkey `openssl rand -base64 32`" > /etc/hostname.wg0
> +client# # sh /etc/netstart wg0
> +clinet# ifconfig wg0 | grep wgpubkey | cut -d ‘ ‘ -f 2
> +f+wtDqJrNnSIRqOCCiBa4YWGZ58WLSo5b5oWjBQt6Xg=
> +client# ifconfig wg0 inet 192.0.2.2/24
> +client# ifconfig wg0 wgpeer zQfRbQPcQiLppc55LJWbFCdrnHdpxZTg47VQmJG6heE= \
> +wgpsk jPNozq8SGbXk5ACrfAHEA3/O1jWlrhiCJ4ibvon3oqc= \
> +wgendpoint a.b.c.d 51820 wgaip 0.0.0.0/0
> +
> +
> +
> +PF rules needed for traffic to enter and leave on the wg0 interface on the
> +client.
> +
> +
> +pass on wg keep state (if-bound)
> +
> +
> +
> +Routes needed on client to tunnel all traffic to / through the server.
> +
> +
> +client# route add -priority 2 a.b.c.d `netstat -rn | grep default | \
> +awk ‘{print $2}’`
> +client# route add -priority 7 default 192.0.2.1/24
> +
> +
> +Server to Server example
> +
> +
> +This example shows 2 servers each with another subnet connected to them. Pay
> +close attention to the wgaip info. There can be multiple allowed IP's or
> +subnets each with their own wgaip for the wgpeer line.
> +
> +
> +Server 1 configuration (initiator and responder)
> +
> +
> +Create a persistnent private key and interface, set listening port, get the
> +pubkey that server 2 will need.
> +
> +
> +server1# echo "wgkey `openssl rand -base64 32`" > /etc/host

sdhc(4) attachment for Zynq-7000

[Visa Hankala]

Zynq-7000 has a variant of Arasan SD controller that is not recognized
by the fdt glue. The diff below fixes this.

The controller's capabilities register lacks the base clock frequency.
The attachment glue has to provide this parameter.

OK?

Index: dev/fdt/sdhc_fdt.c
===
RCS file: src/sys/dev/fdt/sdhc_fdt.c,v
retrieving revision 1.15
diff -u -p -r1.15 sdhc_fdt.c
--- dev/fdt/sdhc_fdt.c  5 Apr 2021 09:31:45 -   1.15
+++ dev/fdt/sdhc_fdt.c  2 May 2021 14:07:45 -
@@ -126,6 +126,7 @@ sdhc_fdt_match(struct device *parent, vo
struct fdt_attach_args *faa = aux;
 
return (OF_is_compatible(faa->fa_node, "arasan,sdhci-5.1") ||
+   OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a") ||
OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2") ||
OF_is_compatible(faa->fa_node, "brcm,bcm2835-sdhci") ||
OF_is_compatible(faa->fa_node, "marvell,armada-3700-sdhci") ||
@@ -232,6 +233,11 @@ sdhc_fdt_attach(struct device *parent, s
sc->sc.sc_flags |= SDHC_F_NODDR50;
}
 
+   if (OF_is_compatible(faa->fa_node, "arasan,sdhci-8.9a")) {
+   freq = clock_get_frequency(faa->fa_node, "clk_xin");
+   sc->sc.sc_clkbase = (freq + 500) / 1000;
+   }
+
if (OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2"))
sc->sc.sc_flags |= SDHC_F_NOPWR0;
 

Re: pthread_once fix memory leak

[Otto Moerbeek]

On Sun, May 02, 2021 at 02:07:21PM +0200, Mark Kettenis wrote:

> > From: Martijn van Duren 
> > Date: Sun, 02 May 2021 13:28:10 +0200
> > 
> > Found this while tracing a memory leak in filter-dkimsign, thanks to
> > libcrypto. The mutex in pthread_once_t is never destroyed, so the
> > memory allocated inside the mutex is never released.
> > 
> > The diff below was inspired by Ed Schouten and switches form mutex to
> > futex to prevent any memory allocation. I've run with it for about a
> > week without issues and tb@ has given it some beating on sparc64.
> > However I'm no expert in this area and scrutiny from people with more
> > experience in this area and testing in general would be appreciated.
> > 
> > This implementation has one shortcoming I can see, namely[0]:
> > The pthread_once() function is not a cancellation point. However, if
> > init_routine is a cancellation point and is canceled, the effect on
> > once_control shall be as if pthread_once() was never called.
> > It doesn't handle this situation by waking up the sleeping threads.
> > However, the current code doesn't handle this requirement either:
> > #include 
> > #include 
> > 
> > pthread_once_t once = PTHREAD_ONCE_INIT;
> > 
> > void
> > init(void)
> > {
> > printf("init\n");
> > pthread_exit(NULL);
> > }
> > 
> > void *
> > routine(void *arg)
> > {
> > pthread_once(&once, init);
> > printf("%s\n", __func__);
> > return NULL;
> > }
> > 
> > int
> > main(int argc, char *argv[])
> > {
> > pthread_t thread;
> > pthread_create(&thread, NULL, routine, NULL);
> > pthread_once(&once, init);
> > printf("%s\n", __func__);
> > return 0;
> > }
> > 
> > Since our current code shows similar behaviour without real world
> > problems and all the solutions that I can come up with are racey I think 
> > this diff can stand on its own and some other brave soul can fix this
> > requirement at a later time. :-)
> > 
> > OK?
> 
> Sorry, no, this is an ABI break.  And a libpthreads major bump is a
> major flag day.
> 
> I don't think this is worth fixing on its own.  There are other
> instances where using a mutex will leak memory.  We need to change the
> mutex implementation such that it doesn't use malloc.  This is needed
> for process shared mutexes too.

Agreed. This is a one-time leak, since once_control must not be on
the stack. So not a big issue. I would love to see malloc-free mutexes
as well.

-Otto

> 
> > Index: include/pthread.h
> > ===
> > RCS file: /cvs/src/include/pthread.h,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 pthread.h
> > --- include/pthread.h   5 Mar 2018 01:15:26 -   1.4
> > +++ include/pthread.h   2 May 2021 11:24:17 -
> > @@ -136,20 +136,13 @@ typedef void  *(*pthread_startroutine_t)(
> >   * Once definitions.
> >   */
> >  struct pthread_once {
> > -   int state;
> > -   pthread_mutex_t mutex;
> > +   volatile unsigned int   state;
> >  };
> >  
> >  /*
> > - * Flags for once initialization.
> > - */
> > -#define PTHREAD_NEEDS_INIT  0
> > -#define PTHREAD_DONE_INIT   1
> > -
> > -/*
> >   * Static once initialization values. 
> >   */
> > -#define PTHREAD_ONCE_INIT   { PTHREAD_NEEDS_INIT, 
> > PTHREAD_MUTEX_INITIALIZER }
> > +#define PTHREAD_ONCE_INIT   { 0 }
> >  
> >  /*
> >   * Static initialization values. 
> > Index: lib/libc/thread/rthread_once.c
> > ===
> > RCS file: /cvs/src/lib/libc/thread/rthread_once.c,v
> > retrieving revision 1.3
> > diff -u -p -r1.3 rthread_once.c
> > --- lib/libc/thread/rthread_once.c  4 Nov 2017 22:53:57 -   1.3
> > +++ lib/libc/thread/rthread_once.c  2 May 2021 11:24:17 -
> > @@ -18,15 +18,25 @@
> >  
> >  #include 
> >  
> > +#include "synch.h"
> > +
> >  int
> >  pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
> >  {
> > -   pthread_mutex_lock(&once_control->mutex);
> > -   if (once_control->state == PTHREAD_NEEDS_INIT) {
> > +   switch (atomic_cas_uint(&(once_control->state), 0, 1)) {
> > +   case 0:
> > init_routine();
> > -   once_control->state = PTHREAD_DONE_INIT;
> > +   atomic_inc_int(&once_control->state);
> > +   _wake(&once_control->state, INT_MAX);
> > +   break;
> > +   case 1:
> > +   do {
> > +   _twait(&once_control->state, 1, 0, NULL);
> > +   } while (once_control->state != 2);
> > +   break;
> > +   default:
> > +   break;
> > }
> > -   pthread_mutex_unlock(&once_control->mutex);
> >  
> > -   return (0);
> > +   return 0;
> >  }
> > 
> > 
> > 
> 

Re: pthread_once fix memory leak

[Mark Kettenis]

> From: Martijn van Duren 
> Date: Sun, 02 May 2021 13:28:10 +0200
> 
> Found this while tracing a memory leak in filter-dkimsign, thanks to
> libcrypto. The mutex in pthread_once_t is never destroyed, so the
> memory allocated inside the mutex is never released.
> 
> The diff below was inspired by Ed Schouten and switches form mutex to
> futex to prevent any memory allocation. I've run with it for about a
> week without issues and tb@ has given it some beating on sparc64.
> However I'm no expert in this area and scrutiny from people with more
> experience in this area and testing in general would be appreciated.
> 
> This implementation has one shortcoming I can see, namely[0]:
> The pthread_once() function is not a cancellation point. However, if
> init_routine is a cancellation point and is canceled, the effect on
> once_control shall be as if pthread_once() was never called.
> It doesn't handle this situation by waking up the sleeping threads.
> However, the current code doesn't handle this requirement either:
> #include 
> #include 
> 
> pthread_once_t once = PTHREAD_ONCE_INIT;
> 
> void
> init(void)
> {
>   printf("init\n");
>   pthread_exit(NULL);
> }
> 
> void *
> routine(void *arg)
> {
>   pthread_once(&once, init);
>   printf("%s\n", __func__);
>   return NULL;
> }
> 
> int
> main(int argc, char *argv[])
> {
>   pthread_t thread;
>   pthread_create(&thread, NULL, routine, NULL);
>   pthread_once(&once, init);
>   printf("%s\n", __func__);
>   return 0;
> }
> 
> Since our current code shows similar behaviour without real world
> problems and all the solutions that I can come up with are racey I think 
> this diff can stand on its own and some other brave soul can fix this
> requirement at a later time. :-)
> 
> OK?

Sorry, no, this is an ABI break.  And a libpthreads major bump is a
major flag day.

I don't think this is worth fixing on its own.  There are other
instances where using a mutex will leak memory.  We need to change the
mutex implementation such that it doesn't use malloc.  This is needed
for process shared mutexes too.

> Index: include/pthread.h
> ===
> RCS file: /cvs/src/include/pthread.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 pthread.h
> --- include/pthread.h 5 Mar 2018 01:15:26 -   1.4
> +++ include/pthread.h 2 May 2021 11:24:17 -
> @@ -136,20 +136,13 @@ typedef void*(*pthread_startroutine_t)(
>   * Once definitions.
>   */
>  struct pthread_once {
> - int state;
> - pthread_mutex_t mutex;
> + volatile unsigned int   state;
>  };
>  
>  /*
> - * Flags for once initialization.
> - */
> -#define PTHREAD_NEEDS_INIT  0
> -#define PTHREAD_DONE_INIT   1
> -
> -/*
>   * Static once initialization values. 
>   */
> -#define PTHREAD_ONCE_INIT   { PTHREAD_NEEDS_INIT, PTHREAD_MUTEX_INITIALIZER }
> +#define PTHREAD_ONCE_INIT   { 0 }
>  
>  /*
>   * Static initialization values. 
> Index: lib/libc/thread/rthread_once.c
> ===
> RCS file: /cvs/src/lib/libc/thread/rthread_once.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 rthread_once.c
> --- lib/libc/thread/rthread_once.c4 Nov 2017 22:53:57 -   1.3
> +++ lib/libc/thread/rthread_once.c2 May 2021 11:24:17 -
> @@ -18,15 +18,25 @@
>  
>  #include 
>  
> +#include "synch.h"
> +
>  int
>  pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
>  {
> - pthread_mutex_lock(&once_control->mutex);
> - if (once_control->state == PTHREAD_NEEDS_INIT) {
> + switch (atomic_cas_uint(&(once_control->state), 0, 1)) {
> + case 0:
>   init_routine();
> - once_control->state = PTHREAD_DONE_INIT;
> + atomic_inc_int(&once_control->state);
> + _wake(&once_control->state, INT_MAX);
> + break;
> + case 1:
> + do {
> + _twait(&once_control->state, 1, 0, NULL);
> + } while (once_control->state != 2);
> + break;
> + default:
> + break;
>   }
> - pthread_mutex_unlock(&once_control->mutex);
>  
> - return (0);
> + return 0;
>  }
> 
> 
> 

pthread_once fix memory leak

[Martijn van Duren]

Found this while tracing a memory leak in filter-dkimsign, thanks to
libcrypto. The mutex in pthread_once_t is never destroyed, so the
memory allocated inside the mutex is never released.

The diff below was inspired by Ed Schouten and switches form mutex to
futex to prevent any memory allocation. I've run with it for about a
week without issues and tb@ has given it some beating on sparc64.
However I'm no expert in this area and scrutiny from people with more
experience in this area and testing in general would be appreciated.

This implementation has one shortcoming I can see, namely[0]:
The pthread_once() function is not a cancellation point. However, if
init_routine is a cancellation point and is canceled, the effect on
once_control shall be as if pthread_once() was never called.
It doesn't handle this situation by waking up the sleeping threads.
However, the current code doesn't handle this requirement either:
#include 
#include 

pthread_once_t once = PTHREAD_ONCE_INIT;

void
init(void)
{
printf("init\n");
pthread_exit(NULL);
}

void *
routine(void *arg)
{
pthread_once(&once, init);
printf("%s\n", __func__);
return NULL;
}

int
main(int argc, char *argv[])
{
pthread_t thread;
pthread_create(&thread, NULL, routine, NULL);
pthread_once(&once, init);
printf("%s\n", __func__);
return 0;
}

Since our current code shows similar behaviour without real world
problems and all the solutions that I can come up with are racey I think 
this diff can stand on its own and some other brave soul can fix this
requirement at a later time. :-)

OK?

martijn@

[0] https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_once.html

Index: include/pthread.h
===
RCS file: /cvs/src/include/pthread.h,v
retrieving revision 1.4
diff -u -p -r1.4 pthread.h
--- include/pthread.h   5 Mar 2018 01:15:26 -   1.4
+++ include/pthread.h   2 May 2021 11:24:17 -
@@ -136,20 +136,13 @@ typedef void  *(*pthread_startroutine_t)(
  * Once definitions.
  */
 struct pthread_once {
-   int state;
-   pthread_mutex_t mutex;
+   volatile unsigned int   state;
 };
 
 /*
- * Flags for once initialization.
- */
-#define PTHREAD_NEEDS_INIT  0
-#define PTHREAD_DONE_INIT   1
-
-/*
  * Static once initialization values. 
  */
-#define PTHREAD_ONCE_INIT   { PTHREAD_NEEDS_INIT, PTHREAD_MUTEX_INITIALIZER }
+#define PTHREAD_ONCE_INIT   { 0 }
 
 /*
  * Static initialization values. 
Index: lib/libc/thread/rthread_once.c
===
RCS file: /cvs/src/lib/libc/thread/rthread_once.c,v
retrieving revision 1.3
diff -u -p -r1.3 rthread_once.c
--- lib/libc/thread/rthread_once.c  4 Nov 2017 22:53:57 -   1.3
+++ lib/libc/thread/rthread_once.c  2 May 2021 11:24:17 -
@@ -18,15 +18,25 @@
 
 #include 
 
+#include "synch.h"
+
 int
 pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
 {
-   pthread_mutex_lock(&once_control->mutex);
-   if (once_control->state == PTHREAD_NEEDS_INIT) {
+   switch (atomic_cas_uint(&(once_control->state), 0, 1)) {
+   case 0:
init_routine();
-   once_control->state = PTHREAD_DONE_INIT;
+   atomic_inc_int(&once_control->state);
+   _wake(&once_control->state, INT_MAX);
+   break;
+   case 1:
+   do {
+   _twait(&once_control->state, 1, 0, NULL);
+   } while (once_control->state != 2);
+   break;
+   default:
+   break;
}
-   pthread_mutex_unlock(&once_control->mutex);
 
-   return (0);
+   return 0;
 }