From: "Jonathan Armani" Subject: Re: sndiod: simplify unix socket binding code To: "Alexandre Ratchov" , tech@openbsd.org Date: Sat, 14 Mar 2026 15:48:15 +0100 ok On Fri, Mar 13, 2026, at 12:35, Alexandre Ratchov wrote: > - 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 > > /* > - * 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))