From: Stuart Henderson Subject: Re: lang/node & net/libcares weirdness To: Volker Schlecht Cc: ports@openbsd.org, brad@comstyle.com, openbsd.ports@aisha.cc Date: Wed, 8 May 2024 00:47:17 +0100 On 2024/05/08 01:14, Volker Schlecht wrote: > aisha@ identified a rather recent problem with lang/node, as in the following > would immediately crash (nevermind the node version. It's 100% reproducible > in 7.5 and -current): > > $ node > Welcome to Node.js v20.12.2. > Type ".help" for more information. > > require('dns').resolve4('openbsd.org','A',(err, records) => {console.log(records);}); > > I tracked this back to the following pull request in libcares: > > https://github.com/c-ares/c-ares/pull/659 So, specifically, this patch is setting SOCK_DNS for the situation where somebody using this library _might_ use pledge, but node is not using pledge. > Reverting that fixes things ... diff attached, which includes an update to 1.28.1, > the version I've been testing with. My reading of kern/sys_generic.c:sys_ioctl is that you can't use ioctl on a SOCK_DNS (and thus SS_DNS) socket. Running node under ktrace and entering your js command I get this shortly afterwards: 37583 node STRU struct sockaddr { AF_INET, 127.0.0.1:53 } 37583 node RET connect 0 37583 node CALL kbind(0x7189e9088988,24,0x884d591d8b972634) 37583 node RET kbind 0 37583 node CALL kbind(0x7189e9088988,24,0x884d591d8b972634) 37583 node RET kbind 0 37583 node CALL kevent(11,0x7189e90889c8,1,0,0,0) 37583 node STRU struct kevent { ident=22, filter=EVFILT_READ, flags=0x1, fflags=0<>, data=0, udata=0x0 } 37583 node RET kevent 0 37583 node CALL kevent(11,0x7189e90889c8,1,0,0,0) 37583 node STRU struct kevent { ident=22, filter=EVFILT_READ, flags=0x2, fflags=0<>, data=0, udata=0x0 } 37583 node RET kevent 0 37583 node CALL ioctl(22,FIONBIO,0x7189e90889f4) 37583 node RET ioctl -1 errno 22 Invalid argument printf debugging in sys_ioctl would confirm but I think it must be hitting that case, and then c-ares and/or node doesn't handle the EINVAL. Your patch to backout this commit seems the correct approach to me. I think adding a comment to the patch would be helpful (I'd include the js command to reproduce the crash). > Index: Makefile > =================================================================== > RCS file: /cvs/ports/net/libcares/Makefile,v > diff -u -p -r1.28 Makefile > --- Makefile 5 Apr 2024 20:38:05 -0000 1.28 > +++ Makefile 7 May 2024 23:11:14 -0000 > @@ -1,12 +1,12 @@ > COMMENT= asynchronous resolver library > > -V= 1.28.0 > +V= 1.28.1 The update looks worth having anyway, but I'd do that as a separate commit. > DISTNAME= c-ares-${V} > PKGNAME= libcares-${V} > CATEGORIES= net devel > SITES= ${HOMEPAGE}download/ > > -SHARED_LIBS += cares 3.6 # 8.1.6 > +SHARED_LIBS += cares 3.7 # 8.1.6 I didn't see a reason to bump this. > HOMEPAGE= https://c-ares.haxx.se/ > > Index: distinfo > =================================================================== > RCS file: /cvs/ports/net/libcares/distinfo,v > diff -u -p -r1.16 distinfo > --- distinfo 5 Apr 2024 20:38:05 -0000 1.16 > +++ distinfo 7 May 2024 23:11:14 -0000 > @@ -1,2 +1,2 @@ > -SHA256 (c-ares-1.28.0.tar.gz) = PJL+u969/p3qDgNwg/VTV3ReDfNCQ/lmDu2HV9thdDA= > -SIZE (c-ares-1.28.0.tar.gz) = 1311900 > +SHA256 (c-ares-1.28.1.tar.gz) = Z1pp/FTdv0LmgwvGce62zYnuykOCjrQTJD/SwKdggJ0= > +SIZE (c-ares-1.28.1.tar.gz) = 1312102 > Index: patches/patch-src_lib_ares__socket_c > =================================================================== > RCS file: patches/patch-src_lib_ares__socket_c > diff -N patches/patch-src_lib_ares__socket_c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ patches/patch-src_lib_ares__socket_c 7 May 2024 23:11:14 -0000 > @@ -0,0 +1,16 @@ > +Index: src/lib/ares__socket.c > +--- src/lib/ares__socket.c.orig > ++++ src/lib/ares__socket.c > +@@ -253,12 +253,6 @@ ares_status_t ares__open_connection(ares_channel_t > + struct server_connection *conn; > + ares__llist_node_t *node; > + int type = is_tcp ? SOCK_STREAM : SOCK_DGRAM; > +-#ifdef __OpenBSD__ > +- if ((is_tcp && server->tcp_port == 53) || > +- (!is_tcp && server->udp_port == 53)) { > +- type |= SOCK_DNS; > +- } > +-#endif > + > + switch (server->addr.family) { > + case AF_INET: