NetBSD Problem Report #56531

From www@netbsd.org  Wed Dec  1 21:29:42 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 7A6FE1A921F
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  1 Dec 2021 21:29:42 +0000 (UTC)
Message-Id: <20211201212941.6346D1A9239@mollari.NetBSD.org>
Date: Wed,  1 Dec 2021 21:29:41 +0000 (UTC)
From: anthony.mallet@laas.fr
Reply-To: anthony.mallet@laas.fr
To: gnats-bugs@NetBSD.org
Subject: Huge buffer stack in getaddrinfo(3)
X-Send-Pr-Version: www-1.0

>Number:         56531
>Category:       lib
>Synopsis:       Huge buffer stack in getaddrinfo(3)
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Dec 01 21:30:00 +0000 2021
>Originator:     Anthony Mallet
>Release:        -current
>Organization:
>Environment:
NetBSD ficus 9.99.91 NetBSD 9.99.91 (FICUS) #31: Sat Oct 16 15:04:36 CEST 2021  troot@ficus:/usr/obj/sys/arch/amd64/compile/FICUS amd64

>Description:
There is a 64kB buffer allocated on the stack in the res_queryN function used by getaddrinfo(3).
https://anonhg.netbsd.org/src/file/tip/lib/libc/net/getaddrinfo.c#l2550

While this is in general not an issue, it can become troublesome in a multi-threaded context with a small custom stack size. While the stack size can be increased, it is a bit unusual to allocate such a big buffer on the stack.

This was discussed in tech-userlevel
http://mail-index.netbsd.org/tech-userlevel/2021/11/28/msg013174.html

In rev. 1.63:
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/net/getaddrinfo.c.diff?r1=1.62&r2=1.63
I believe that the intent of the commit was to increase the buffer size used to read DNS replies. As a (probably unwanted) side effect, this modified the buffer size used only to send queries.

This may be confirmed by the fact that a very similar function res_nquery()
https://anonhg.netbsd.org/src/file/tip/lib/libc/resolv/res_query.c#l145
does the same thing with a smaller buffer.

So I think it should be shrink the buffer used in res_queryN to the size it was before rev. 1.63 of getaddrinfo.c, as suggested in the attached patch.

If it is believed that the buffer should remain at 64kB, I also can provide a patch that allocates the buffer on the heap instead.


>How-To-Repeat:

>Fix:
Index: getaddrinfo.c
===================================================================
RCS file: /cvsroot/src/lib/libc/net/getaddrinfo.c,v
retrieving revision 1.122
diff -u -r1.122 getaddrinfo.c
--- getaddrinfo.c	28 Oct 2021 20:56:32 -0000	1.122
+++ getaddrinfo.c	29 Nov 2021 15:56:07 -0000
@@ -211,6 +211,11 @@
 	{ 0, 0 }
 };

+#if PACKETSZ > 1024
+#define MAXQUERY	PACKETSZ
+#else
+#define MAXQUERY	1024
+#endif
 #define MAXPACKET	(64*1024)

 typedef union {
@@ -2547,7 +2552,7 @@
 res_queryN(const char *name, /* domain name */ struct res_target *target,
     res_state statp)
 {
-	u_char buf[MAXPACKET];
+	u_char buf[MAXQUERY];
 	HEADER *hp;
 	int n;
 	struct res_target *t;

Home	PR Database Search