Index | Thread | Search

From:
Alexandre Ratchov <alex@caoua.org>
Subject:
sndiod: simplify unix socket binding code
To:
tech@openbsd.org
Date:
Fri, 13 Mar 2026 12:35:46 +0100

Download raw body.

Thread
  • Alexandre Ratchov:

    sndiod: simplify unix socket binding code

- Use sizeof(sockaddr_un->sun_path) instead of the complicated
  calculation of the max socket path length.

- Use the socket's address family to determine if this is an Unix
  socket: no need to store the path, allocate memory for it, then
  remember to free it.

OK?

Index: lib/libsndio/amsg.h
===================================================================
RCS file: /cvs/src/lib/libsndio/amsg.h,v
diff -u -p -u -p -r1.17 amsg.h
--- lib/libsndio/amsg.h	22 Jan 2026 09:24:26 -0000	1.17
+++ lib/libsndio/amsg.h	13 Mar 2026 11:31:29 -0000
@@ -20,22 +20,10 @@
 #include <stdint.h>
 
 /*
- * unix-domain socket name is:
- *
- * DIR [ '-' UID ] '/' FILE UNIT
- *
- * example: "/tmp/sndio-1000/sock0"
- *
+ * unix-domain socket (example: "/tmp/sndio-1000/sock0")
  */
 #define SOCKPATH_DIR	"/tmp/sndio"
 #define SOCKPATH_FILE	"sock"
-#define SOCKPATH_MAX	(1 +		\
-	sizeof(SOCKPATH_DIR) - 1 +	\
-	sizeof(char) +			\
-	sizeof(int) * 3 +		\
-	sizeof(char) +			\
-	sizeof(SOCKPATH_FILE) - 1 +	\
-	sizeof(int) * 3)
 
 /*
  * server TCP base port number
Index: usr.bin/sndiod/listen.c
===================================================================
RCS file: /cvs/src/usr.bin/sndiod/listen.c,v
diff -u -p -u -p -r1.16 listen.c
--- usr.bin/sndiod/listen.c	15 Sep 2025 08:39:22 -0000	1.16
+++ usr.bin/sndiod/listen.c	13 Mar 2026 11:31:29 -0000
@@ -67,40 +67,81 @@ listen_close(struct listen *f)
 	}
 	*pf = f->next;
 
-	if (f->path != NULL) {
-		xfree(f->path);
-	}
 	file_del(f->file);
 	close(f->fd);
 	xfree(f);
 }
 
 int
-listen_new_un(char *path)
+listen_new_un(unsigned int unit)
 {
-	int sock, oldumask;
+	int len, sock, oldumask;
 	struct sockaddr_un sockname;
 	struct listen *f;
+	struct stat sb;
+	uid_t uid;
+	mode_t mask, omask;
+	char dir[sizeof(sockname.sun_path)];
+
+	uid = geteuid();
+	if (uid == 0) {
+		mask = 022;
+		len = snprintf(sockname.sun_path, sizeof(sockname.sun_path),
+		    SOCKPATH_DIR "/" SOCKPATH_FILE "%u", unit);
+	} else {
+		mask = 077;
+		len = snprintf(sockname.sun_path, sizeof(sockname.sun_path),
+		    SOCKPATH_DIR "-%u/" SOCKPATH_FILE "%u", uid, unit);
+	}
+	if (len >= sizeof(sockname.sun_path)) {
+		logx(0, "unix socket name too long");
+		return 0;
+	}
+
+	while (sockname.sun_path[len] != '/')
+		len--;
+	memcpy(dir, sockname.sun_path, len);
+	dir[len] = 0;
+
+	omask = umask(mask);
+	if (mkdir(dir, 0777) == -1) {
+		if (errno != EEXIST) {
+			logx(0, "mkdir(\"%s\")", dir);
+			return 0;
+		}
+	}
+	umask(omask);
+	if (stat(dir, &sb) == -1) {
+		logx(0, "stat(\"%s\")", dir);
+		return 0;
+	}
+	if (!S_ISDIR(sb.st_mode)) {
+		logx(0, "%s is not a directory", dir);
+		return 0;
+	}
+	if (sb.st_uid != uid || (sb.st_mode & mask) != 0) {
+		logx(0, "%s has wrong permissions", dir);
+		return 0;
+	}
 
 	sock = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (sock == -1) {
-		logx(0, "%s: failed to create socket", path);
+		logx(0, "%s: failed to create socket", sockname.sun_path);
 		return 0;
 	}
-	if (unlink(path) == -1 && errno != ENOENT) {
-		logx(0, "%s: failed to unlink socket", path);
+	if (unlink(sockname.sun_path) == -1 && errno != ENOENT) {
+		logx(0, "%s: failed to unlink socket", sockname.sun_path);
 		goto bad_close;
 	}
 	sockname.sun_family = AF_UNIX;
-	strlcpy(sockname.sun_path, path, sizeof(sockname.sun_path));
 	oldumask = umask(0111);
 	if (bind(sock, (struct sockaddr *)&sockname,
 		sizeof(struct sockaddr_un)) == -1) {
-		logx(0, "%s: failed to bind socket", path);
+		logx(0, "%s: failed to bind socket", sockname.sun_path);
 		goto bad_close;
 	}
 	if (listen(sock, 1) == -1) {
-		logx(0, "%s: failed to listen", path);
+		logx(0, "%s: failed to listen", sockname.sun_path);
 		goto bad_close;
 	}
 	umask(oldumask);
@@ -108,7 +149,6 @@ listen_new_un(char *path)
 	f->file = file_new(&listen_fileops, f, "unix", 1);
 	if (f->file == NULL)
 		goto bad_close;
-	f->path = xstrdup(path);
 	f->fd = sock;
 	f->next = listen_list;
 	listen_list = f;
@@ -172,7 +212,6 @@ listen_new_tcp(char *addr, unsigned int 
 			close(s);
 			continue;
 		}
-		f->path = NULL;
 		f->fd = s;
 		f->next = listen_list;
 		listen_list = f;
@@ -231,7 +270,7 @@ listen_in(void *arg)
 		logx(0, "%s: failed to set non-blocking mode", f->file->name);
 		goto bad_close;
 	}
-	if (f->path == NULL) {
+	if (caddr.sa_family == AF_INET || caddr.sa_family == AF_INET6) {
 		opt = 1;
 		if (setsockopt(sock, IPPROTO_TCP, TCP_NODELAY,
 		    &opt, sizeof(int)) == -1) {
Index: usr.bin/sndiod/listen.h
===================================================================
RCS file: /cvs/src/usr.bin/sndiod/listen.h,v
diff -u -p -u -p -r1.3 listen.h
--- usr.bin/sndiod/listen.h	8 Jan 2016 13:14:11 -0000	1.3
+++ usr.bin/sndiod/listen.h	13 Mar 2026 11:31:29 -0000
@@ -22,14 +22,13 @@ struct file;
 struct listen {
 	struct listen *next;
 	struct file *file;
-	char *path;
 	int fd;
 	int slowaccept;
 };
 
 extern struct listen *listen_list;
 
-int listen_new_un(char *);
+int listen_new_un(unsigned int);
 int listen_new_tcp(char *, unsigned int);
 int listen_init(struct listen *);
 void listen_close(struct listen *);
Index: usr.bin/sndiod/sndiod.c
===================================================================
RCS file: /cvs/src/usr.bin/sndiod/sndiod.c,v
diff -u -p -u -p -r1.52 sndiod.c
--- usr.bin/sndiod/sndiod.c	26 Nov 2025 08:40:16 -0000	1.52
+++ usr.bin/sndiod/sndiod.c	13 Mar 2026 11:31:29 -0000
@@ -364,35 +364,6 @@ unsetsig(void)
 		err(1, "unsetsig(int): sigaction failed");
 }
 
-void
-getbasepath(char *base)
-{
-	uid_t uid;
-	struct stat sb;
-	mode_t mask, omask;
-
-	uid = geteuid();
-	if (uid == 0) {
-		mask = 022;
-		snprintf(base, SOCKPATH_MAX, SOCKPATH_DIR);
-	} else {
-		mask = 077;
-		snprintf(base, SOCKPATH_MAX, SOCKPATH_DIR "-%u", uid);
-	}
-	omask = umask(mask);
-	if (mkdir(base, 0777) == -1) {
-		if (errno != EEXIST)
-			err(1, "mkdir(\"%s\")", base);
-	}
-	umask(omask);
-	if (stat(base, &sb) == -1)
-		err(1, "stat(\"%s\")", base);
-	if (!S_ISDIR(sb.st_mode))
-		errx(1, "%s is not a directory", base);
-	if (sb.st_uid != uid || (sb.st_mode & mask) != 0)
-		errx(1, "%s has wrong permissions", base);
-}
-
 struct dev *
 mkdev(char *path, struct aparams *par,
     int mode, int bufsz, int round, int rate, int hold, int autovol)
@@ -536,7 +507,6 @@ main(int argc, char **argv)
 {
 	int c, i, background, unit;
 	int pmin, pmax, rmin, rmax;
-	char base[SOCKPATH_MAX], path[SOCKPATH_MAX];
 	unsigned int mode, dup, mmc, vol;
 	unsigned int hold, autovol, bufsz, round, rate;
 	const char *str;
@@ -750,9 +720,7 @@ main(int argc, char **argv)
 			errx(1, "unknown user %s", SNDIO_USER);
 	} else
 		pw = NULL;
-	getbasepath(base);
-	snprintf(path, SOCKPATH_MAX, "%s/" SOCKPATH_FILE "%u", base, unit);
-	if (!listen_new_un(path))
+	if (!listen_new_un(unit))
 		return 1;
 	for (ta = tcpaddr_list; ta != NULL; ta = ta->next) {
 		if (!listen_new_tcp(ta->host, AUCAT_PORT + unit))