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<EV_ADD>, 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<EV_DELETE>, 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: