* [pushed 2.35 01/13] nss: Sort tests and tests-container and put one test per line
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 02/13] gaih_inet: Simplify canon name resolution Siddhesh Poyarekar
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit e2f68b54e8052da14680074fc5df03153216f218)
---
nss/Makefile | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/nss/Makefile b/nss/Makefile
index b2301c4cd7..d8b06b44fb 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -56,22 +56,31 @@ extra-objs += $(makedb-modules:=.o)
tests-static = tst-field
tests-internal = tst-field
-tests = test-netdb test-digits-dots tst-nss-getpwent bug17079 \
- tst-nss-test1 \
- tst-nss-test2 \
- tst-nss-test4 \
- tst-nss-test5 \
- tst-nss-test_errno
-xtests = bug-erange
-
-tests-container = \
- tst-nss-compat1 \
- tst-nss-test3 \
- tst-nss-files-hosts-long \
- tst-nss-db-endpwent \
- tst-nss-db-endgrent \
- tst-reload1 tst-reload2 \
- tst-nss-gai-actions
+
+tests := \
+ bug17079 \
+ test-digits-dots \
+ test-netdb \
+ tst-nss-getpwent \
+ tst-nss-test1 \
+ tst-nss-test2 \
+ tst-nss-test4 \
+ tst-nss-test5 \
+ tst-nss-test_errno \
+# tests
+
+xtests = bug-erange
+
+tests-container := \
+ tst-nss-compat1 \
+ tst-nss-db-endgrent \
+ tst-nss-db-endpwent \
+ tst-nss-files-hosts-long \
+ tst-nss-gai-actions \
+ tst-nss-test3 \
+ tst-reload1 \
+ tst-reload2 \
+# tests-container
# Tests which need libdl
ifeq (yes,$(build-shared))
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 02/13] gaih_inet: Simplify canon name resolution
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 01/13] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 03/13] getaddrinfo: Fix leak with AI_ALL [BZ #28852] Siddhesh Poyarekar
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Simplify logic for allocation of canon to remove the canonbuf variable;
canon now always points to an allocated block. Also pull the canon name
set into a separate function.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit d01411f6bc61429fc027c38827bf3103b48eef2e)
---
sysdeps/posix/getaddrinfo.c | 130 +++++++++++++++++++++---------------
1 file changed, 75 insertions(+), 55 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 3d9bea60c6..0629fd147b 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -285,7 +285,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
\
if (localcanon != NULL && canon == NULL) \
{ \
- canonbuf = __strdup (localcanon); \
+ char *canonbuf = __strdup (localcanon); \
if (canonbuf == NULL) \
{ \
__resolv_context_put (res_ctx); \
@@ -323,6 +323,41 @@ getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
return __strdup (name);
}
+/* Process looked up canonical name and if necessary, decode to IDNA. Result
+ is a new string written to CANONP and the earlier string is freed. */
+
+static int
+process_canonname (const struct addrinfo *req, const char *orig_name,
+ char **canonp)
+{
+ char *canon = *canonp;
+
+ if ((req->ai_flags & AI_CANONNAME) != 0)
+ {
+ bool do_idn = req->ai_flags & AI_CANONIDN;
+ if (do_idn)
+ {
+ char *out;
+ int rc = __idna_from_dns_encoding (canon ?: orig_name, &out);
+ if (rc == 0)
+ {
+ free (canon);
+ canon = out;
+ }
+ else if (rc == EAI_IDN_ENCODE)
+ /* Use the punycode name as a fallback. */
+ do_idn = false;
+ else
+ return -rc;
+ }
+ if (!do_idn && canon == NULL && (canon = __strdup (orig_name)) == NULL)
+ return -EAI_MEMORY;
+ }
+
+ *canonp = canon;
+ return 0;
+}
+
static int
gaih_inet (const char *name, const struct gaih_service *service,
const struct addrinfo *req, struct addrinfo **pai,
@@ -332,7 +367,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct gaih_servtuple *st = (struct gaih_servtuple *) &nullserv;
struct gaih_addrtuple *at = NULL;
bool got_ipv6 = false;
- const char *canon = NULL;
+ char *canon = NULL;
const char *orig_name = name;
/* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -453,7 +488,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
bool malloc_name = false;
struct gaih_addrtuple *addrmem = NULL;
- char *canonbuf = NULL;
int result = 0;
if (name != NULL)
@@ -495,7 +529,15 @@ gaih_inet (const char *name, const struct gaih_service *service,
}
if (req->ai_flags & AI_CANONNAME)
- canon = name;
+ {
+ char *canonbuf = __strdup (name);
+ if (canonbuf == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto free_and_return;
+ }
+ canon = canonbuf;
+ }
goto process_list;
}
@@ -545,7 +587,15 @@ gaih_inet (const char *name, const struct gaih_service *service,
}
if (req->ai_flags & AI_CANONNAME)
- canon = name;
+ {
+ char *canonbuf = __strdup (name);
+ if (canonbuf == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto free_and_return;
+ }
+ canon = canonbuf;
+ }
goto process_list;
}
@@ -676,9 +726,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
(*pat)->next = NULL;
if (added_canon || air->canon == NULL)
(*pat)->name = NULL;
- else if (canonbuf == NULL)
+ else if (canon == NULL)
{
- canonbuf = __strdup (air->canon);
+ char *canonbuf = __strdup (air->canon);
if (canonbuf == NULL)
{
result = -EAI_MEMORY;
@@ -748,9 +798,9 @@ gaih_inet (const char *name, const struct gaih_service *service,
/* Always start afresh; continue should discard previous results
and the hosts database does not support merge. */
at = NULL;
- free (canonbuf);
+ free (canon);
free (addrmem);
- canon = canonbuf = NULL;
+ canon = NULL;
addrmem = NULL;
got_ipv6 = false;
@@ -805,7 +855,16 @@ gaih_inet (const char *name, const struct gaih_service *service,
no_data = 1;
if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
- canon = at->name;
+ {
+ char *canonbuf = __strdup (at->name);
+ if (canonbuf == NULL)
+ {
+ __resolv_context_put (res_ctx);
+ result = -EAI_MEMORY;
+ goto free_and_return;
+ }
+ canon = canonbuf;
+ }
struct gaih_addrtuple **pat = &at;
@@ -893,7 +952,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
if ((req->ai_flags & AI_CANONNAME) != 0
&& canon == NULL)
{
- canonbuf = getcanonname (nip, at, name);
+ char *canonbuf = getcanonname (nip, at, name);
if (canonbuf == NULL)
{
__resolv_context_put (res_ctx);
@@ -1004,6 +1063,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
}
{
+ /* Set up the canonical name if we need it. */
+ if ((result = process_canonname (req, orig_name, &canon)) != 0)
+ goto free_and_return;
+
struct gaih_servtuple *st2;
struct gaih_addrtuple *at2 = at;
size_t socklen;
@@ -1014,48 +1077,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
*/
while (at2 != NULL)
{
- /* Only the first entry gets the canonical name. */
- if (at2 == at && (req->ai_flags & AI_CANONNAME) != 0)
- {
- if (canon == NULL)
- /* If the canonical name cannot be determined, use
- the passed in string. */
- canon = orig_name;
-
- bool do_idn = req->ai_flags & AI_CANONIDN;
- if (do_idn)
- {
- char *out;
- int rc = __idna_from_dns_encoding (canon, &out);
- if (rc == 0)
- canon = out;
- else if (rc == EAI_IDN_ENCODE)
- /* Use the punycode name as a fallback. */
- do_idn = false;
- else
- {
- result = -rc;
- goto free_and_return;
- }
- }
- if (!do_idn)
- {
- if (canonbuf != NULL)
- /* We already allocated the string using malloc, but
- the buffer is now owned by canon. */
- canonbuf = NULL;
- else
- {
- canon = __strdup (canon);
- if (canon == NULL)
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- }
- }
- }
-
family = at2->family;
if (family == AF_INET6)
{
@@ -1078,7 +1099,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
if (ai == NULL)
{
- free ((char *) canon);
result = -EAI_MEMORY;
goto free_and_return;
}
@@ -1138,7 +1158,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
if (malloc_name)
free ((char *) name);
free (addrmem);
- free (canonbuf);
+ free (canon);
return result;
}
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 03/13] getaddrinfo: Fix leak with AI_ALL [BZ #28852]
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 01/13] nss: Sort tests and tests-container and put one test per line Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 02/13] gaih_inet: Simplify canon name resolution Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 04/13] gaih_inet: Simplify service resolution Siddhesh Poyarekar
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Use realloc in convert_hostent_to_gaih_addrtuple and fix up pointers in
the result list so that a single block is maintained for
hostbyname3_r/hostbyname2_r and freed in gaih_inet. This result is
never merged with any other results, since the hosts database does not
permit merging.
Resolves BZ #28852.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 300460460706ce3ffe29a7df8966e68323ec5bf1)
---
sysdeps/posix/getaddrinfo.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 0629fd147b..e9deb2da6a 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -189,19 +189,16 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
return 0;
}
-/* Convert struct hostent to a list of struct gaih_addrtuple objects.
- h_name is not copied, and the struct hostent object must not be
- deallocated prematurely. *RESULT must be NULL or a pointer to a
- linked-list. The new addresses are appended at the end. */
+/* Convert struct hostent to a list of struct gaih_addrtuple objects. h_name
+ is not copied, and the struct hostent object must not be deallocated
+ prematurely. The new addresses are appended to the tuple array in
+ RESULT. */
static bool
convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
int family,
struct hostent *h,
struct gaih_addrtuple **result)
{
- while (*result)
- result = &(*result)->next;
-
/* Count the number of addresses in h->h_addr_list. */
size_t count = 0;
for (char **p = h->h_addr_list; *p != NULL; ++p)
@@ -212,10 +209,30 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
return true;
- struct gaih_addrtuple *array = calloc (count, sizeof (*array));
+ struct gaih_addrtuple *array = *result;
+ size_t old = 0;
+
+ while (array != NULL)
+ {
+ old++;
+ array = array->next;
+ }
+
+ array = realloc (*result, (old + count) * sizeof (*array));
+
if (array == NULL)
return false;
+ *result = array;
+
+ /* Update the next pointers on reallocation. */
+ for (size_t i = 0; i < old; i++)
+ array[i].next = array + i + 1;
+
+ array += old;
+
+ memset (array, 0, count * sizeof (*array));
+
for (size_t i = 0; i < count; ++i)
{
if (family == AF_INET && req->ai_family == AF_INET6)
@@ -235,7 +252,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
array[0].name = h->h_name;
array[count - 1].next = NULL;
- *result = array;
return true;
}
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 04/13] gaih_inet: Simplify service resolution
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (2 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 03/13] getaddrinfo: Fix leak with AI_ALL [BZ #28852] Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 05/13] gaih_inet: make numeric lookup a separate routine Siddhesh Poyarekar
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Refactor the code to split out the service resolution code into a
separate function. Allocate the service tuples array just once to the
size of the typeproto array, thus avoiding the unnecessary pointer
chasing and stack allocations.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 8d6cf99f2fb81a097f9334c125e5c23604af1a98)
---
sysdeps/posix/getaddrinfo.c | 178 ++++++++++++++++--------------------
1 file changed, 78 insertions(+), 100 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index e9deb2da6a..dae5e9f55f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -100,14 +100,12 @@ struct gaih_service
struct gaih_servtuple
{
- struct gaih_servtuple *next;
int socktype;
int protocol;
int port;
+ bool set;
};
-static const struct gaih_servtuple nullserv;
-
struct gaih_typeproto
{
@@ -180,11 +178,11 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
}
while (r);
- st->next = NULL;
st->socktype = tp->socktype;
st->protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
? req->ai_protocol : tp->protocol);
st->port = s->s_port;
+ st->set = true;
return 0;
}
@@ -375,20 +373,11 @@ process_canonname (const struct addrinfo *req, const char *orig_name,
}
static int
-gaih_inet (const char *name, const struct gaih_service *service,
- const struct addrinfo *req, struct addrinfo **pai,
- unsigned int *naddrs, struct scratch_buffer *tmpbuf)
+get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
+ struct gaih_servtuple *st, struct scratch_buffer *tmpbuf)
{
+ int i;
const struct gaih_typeproto *tp = gaih_inet_typeproto;
- struct gaih_servtuple *st = (struct gaih_servtuple *) &nullserv;
- struct gaih_addrtuple *at = NULL;
- bool got_ipv6 = false;
- char *canon = NULL;
- const char *orig_name = name;
-
- /* Reserve stack memory for the scratch buffer in the getaddrinfo
- function. */
- size_t alloca_used = sizeof (struct scratch_buffer);
if (req->ai_protocol || req->ai_socktype)
{
@@ -410,98 +399,88 @@ gaih_inet (const char *name, const struct gaih_service *service,
}
}
- int port = 0;
- if (service != NULL)
+ if (service != NULL && (tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
+ return -EAI_SERVICE;
+
+ if (service == NULL || service->num >= 0)
{
- if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
- return -EAI_SERVICE;
+ int port = service != NULL ? htons (service->num) : 0;
- if (service->num < 0)
+ if (req->ai_socktype || req->ai_protocol)
{
- if (tp->name[0])
- {
- st = (struct gaih_servtuple *)
- alloca_account (sizeof (struct gaih_servtuple), alloca_used);
-
- int rc = gaih_inet_serv (service->name, tp, req, st, tmpbuf);
- if (__glibc_unlikely (rc != 0))
- return rc;
- }
- else
- {
- struct gaih_servtuple **pst = &st;
- for (tp++; tp->name[0]; tp++)
- {
- struct gaih_servtuple *newp;
+ st[0].socktype = tp->socktype;
+ st[0].protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
+ ? req->ai_protocol : tp->protocol);
+ st[0].port = port;
+ st[0].set = true;
- if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
- continue;
+ return 0;
+ }
- if (req->ai_socktype != 0
- && req->ai_socktype != tp->socktype)
- continue;
- if (req->ai_protocol != 0
- && !(tp->protoflag & GAI_PROTO_PROTOANY)
- && req->ai_protocol != tp->protocol)
- continue;
+ /* Neither socket type nor protocol is set. Return all socket types
+ we know about. */
+ for (i = 0, ++tp; tp->name[0]; ++tp)
+ if (tp->defaultflag)
+ {
+ st[i].socktype = tp->socktype;
+ st[i].protocol = tp->protocol;
+ st[i].port = port;
+ st[i++].set = true;
+ }
- newp = (struct gaih_servtuple *)
- alloca_account (sizeof (struct gaih_servtuple),
- alloca_used);
+ return 0;
+ }
- if (gaih_inet_serv (service->name,
- tp, req, newp, tmpbuf) != 0)
- continue;
+ if (tp->name[0])
+ return gaih_inet_serv (service->name, tp, req, st, tmpbuf);
- *pst = newp;
- pst = &(newp->next);
- }
- if (st == (struct gaih_servtuple *) &nullserv)
- return -EAI_SERVICE;
- }
- }
- else
- {
- port = htons (service->num);
- goto got_port;
- }
- }
- else
+ for (i = 0, tp++; tp->name[0]; tp++)
{
- got_port:
+ if ((tp->protoflag & GAI_PROTO_NOSERVICE) != 0)
+ continue;
- if (req->ai_socktype || req->ai_protocol)
- {
- st = alloca_account (sizeof (struct gaih_servtuple), alloca_used);
- st->next = NULL;
- st->socktype = tp->socktype;
- st->protocol = ((tp->protoflag & GAI_PROTO_PROTOANY)
- ? req->ai_protocol : tp->protocol);
- st->port = port;
- }
- else
- {
- /* Neither socket type nor protocol is set. Return all socket types
- we know about. */
- struct gaih_servtuple **lastp = &st;
- for (++tp; tp->name[0]; ++tp)
- if (tp->defaultflag)
- {
- struct gaih_servtuple *newp;
+ if (req->ai_socktype != 0
+ && req->ai_socktype != tp->socktype)
+ continue;
+ if (req->ai_protocol != 0
+ && !(tp->protoflag & GAI_PROTO_PROTOANY)
+ && req->ai_protocol != tp->protocol)
+ continue;
- newp = alloca_account (sizeof (struct gaih_servtuple),
- alloca_used);
- newp->next = NULL;
- newp->socktype = tp->socktype;
- newp->protocol = tp->protocol;
- newp->port = port;
+ if (gaih_inet_serv (service->name,
+ tp, req, &st[i], tmpbuf) != 0)
+ continue;
- *lastp = newp;
- lastp = &newp->next;
- }
- }
+ i++;
}
+ if (!st[0].set)
+ return -EAI_SERVICE;
+
+ return 0;
+}
+
+static int
+gaih_inet (const char *name, const struct gaih_service *service,
+ const struct addrinfo *req, struct addrinfo **pai,
+ unsigned int *naddrs, struct scratch_buffer *tmpbuf)
+{
+ struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
+ / sizeof (struct gaih_typeproto)] = {0};
+
+ struct gaih_addrtuple *at = NULL;
+ bool got_ipv6 = false;
+ char *canon = NULL;
+ const char *orig_name = name;
+
+ /* Reserve stack memory for the scratch buffer in the getaddrinfo
+ function. */
+ size_t alloca_used = sizeof (struct scratch_buffer);
+
+ int rc;
+ if ((rc = get_servtuples (service, req, st, tmpbuf)) != 0)
+ return rc;
+
bool malloc_name = false;
struct gaih_addrtuple *addrmem = NULL;
int result = 0;
@@ -1083,7 +1062,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
if ((result = process_canonname (req, orig_name, &canon)) != 0)
goto free_and_return;
- struct gaih_servtuple *st2;
struct gaih_addrtuple *at2 = at;
size_t socklen;
sa_family_t family;
@@ -1109,7 +1087,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
else
socklen = sizeof (struct sockaddr_in);
- for (st2 = st; st2 != NULL; st2 = st2->next)
+ for (int i = 0; st[i].set; i++)
{
struct addrinfo *ai;
ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
@@ -1121,8 +1099,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
ai->ai_flags = req->ai_flags;
ai->ai_family = family;
- ai->ai_socktype = st2->socktype;
- ai->ai_protocol = st2->protocol;
+ ai->ai_socktype = st[i].socktype;
+ ai->ai_protocol = st[i].protocol;
ai->ai_addrlen = socklen;
ai->ai_addr = (void *) (ai + 1);
@@ -1144,7 +1122,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct sockaddr_in6 *sin6p =
(struct sockaddr_in6 *) ai->ai_addr;
- sin6p->sin6_port = st2->port;
+ sin6p->sin6_port = st[i].port;
sin6p->sin6_flowinfo = 0;
memcpy (&sin6p->sin6_addr,
at2->addr, sizeof (struct in6_addr));
@@ -1154,7 +1132,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
{
struct sockaddr_in *sinp =
(struct sockaddr_in *) ai->ai_addr;
- sinp->sin_port = st2->port;
+ sinp->sin_port = st[i].port;
memcpy (&sinp->sin_addr,
at2->addr, sizeof (struct in_addr));
memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 05/13] gaih_inet: make numeric lookup a separate routine
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (3 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 04/13] gaih_inet: Simplify service resolution Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 06/13] gaih_inet: Split simple gethostbyname into its own function Siddhesh Poyarekar
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Introduce the gaih_result structure and general paradigm for cleanups
that follow to process the lookup request and return a result. A lookup
function (like text_to_binary_address), should return an integer error
code and set members of gaih_result based on what it finds. If the
function does not have a result and no errors have occurred during the
lookup, it should return 0 and res.at should be set to NULL, allowing a
subsequent function to do the lookup until we run out of options.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 26dea461191cca519b498890a9682fe4bc8e4c2f)
---
sysdeps/posix/getaddrinfo.c | 891 ++++++++++++++++++------------------
1 file changed, 452 insertions(+), 439 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index dae5e9f55f..19bb13db59 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -116,6 +116,12 @@ struct gaih_typeproto
char name[8];
};
+struct gaih_result
+{
+ struct gaih_addrtuple *at;
+ char *canon;
+};
+
/* Values for `protoflag'. */
#define GAI_PROTO_NOSERVICE 1
#define GAI_PROTO_PROTOANY 2
@@ -297,7 +303,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
} \
*pat = addrmem; \
\
- if (localcanon != NULL && canon == NULL) \
+ if (localcanon != NULL && res.canon == NULL) \
{ \
char *canonbuf = __strdup (localcanon); \
if (canonbuf == NULL) \
@@ -306,7 +312,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
result = -EAI_SYSTEM; \
goto free_and_return; \
} \
- canon = canonbuf; \
+ res.canon = canonbuf; \
} \
if (_family == AF_INET6 && *pat != NULL) \
got_ipv6 = true; \
@@ -342,9 +348,9 @@ getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
static int
process_canonname (const struct addrinfo *req, const char *orig_name,
- char **canonp)
+ struct gaih_result *res)
{
- char *canon = *canonp;
+ char *canon = res->canon;
if ((req->ai_flags & AI_CANONNAME) != 0)
{
@@ -368,7 +374,7 @@ process_canonname (const struct addrinfo *req, const char *orig_name,
return -EAI_MEMORY;
}
- *canonp = canon;
+ res->canon = canon;
return 0;
}
@@ -460,6 +466,105 @@ get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
return 0;
}
+/* Convert numeric addresses to binary into RES. On failure, RES->AT is set to
+ NULL and an error code is returned. If AI_NUMERIC_HOST is not requested and
+ the function cannot determine a result, RES->AT is set to NULL and 0
+ returned. */
+
+static int
+text_to_binary_address (const char *name, const struct addrinfo *req,
+ struct gaih_result *res)
+{
+ struct gaih_addrtuple *at = res->at;
+ int result = 0;
+
+ assert (at != NULL);
+
+ memset (at->addr, 0, sizeof (at->addr));
+ if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
+ {
+ if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
+ at->family = AF_INET;
+ else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
+ {
+ at->addr[3] = at->addr[0];
+ at->addr[2] = htonl (0xffff);
+ at->addr[1] = 0;
+ at->addr[0] = 0;
+ at->family = AF_INET6;
+ }
+ else
+ {
+ result = -EAI_ADDRFAMILY;
+ goto out;
+ }
+
+ if (req->ai_flags & AI_CANONNAME)
+ {
+ char *canonbuf = __strdup (name);
+ if (canonbuf == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ res->canon = canonbuf;
+ }
+ return 0;
+ }
+
+ char *scope_delim = strchr (name, SCOPE_DELIMITER);
+ int e;
+
+ if (scope_delim == NULL)
+ e = inet_pton (AF_INET6, name, at->addr);
+ else
+ e = __inet_pton_length (AF_INET6, name, scope_delim - name, at->addr);
+
+ if (e > 0)
+ {
+ if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+ at->family = AF_INET6;
+ else if (req->ai_family == AF_INET
+ && IN6_IS_ADDR_V4MAPPED (at->addr))
+ {
+ at->addr[0] = at->addr[3];
+ at->family = AF_INET;
+ }
+ else
+ {
+ result = -EAI_ADDRFAMILY;
+ goto out;
+ }
+
+ if (scope_delim != NULL
+ && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
+ scope_delim + 1, &at->scopeid) != 0)
+ {
+ result = -EAI_NONAME;
+ goto out;
+ }
+
+ if (req->ai_flags & AI_CANONNAME)
+ {
+ char *canonbuf = __strdup (name);
+ if (canonbuf == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ res->canon = canonbuf;
+ }
+ return 0;
+ }
+
+ if ((req->ai_flags & AI_NUMERICHOST))
+ result = -EAI_NONAME;
+
+out:
+ res->at = NULL;
+ return result;
+}
+
static int
gaih_inet (const char *name, const struct gaih_service *service,
const struct addrinfo *req, struct addrinfo **pai,
@@ -468,9 +573,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
/ sizeof (struct gaih_typeproto)] = {0};
- struct gaih_addrtuple *at = NULL;
bool got_ipv6 = false;
- char *canon = NULL;
const char *orig_name = name;
/* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -485,6 +588,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct gaih_addrtuple *addrmem = NULL;
int result = 0;
+ struct gaih_result res = {0};
if (name != NULL)
{
if (req->ai_flags & AI_IDN)
@@ -497,533 +601,441 @@ gaih_inet (const char *name, const struct gaih_service *service,
malloc_name = true;
}
- uint32_t addr[4];
- if (__inet_aton_exact (name, (struct in_addr *) addr) != 0)
+ res.at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
+ res.at->scopeid = 0;
+ res.at->next = NULL;
+
+ if ((result = text_to_binary_address (name, req, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
+
+ int no_data = 0;
+ int no_inet6_data = 0;
+ nss_action_list nip;
+ enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
+ enum nss_status status = NSS_STATUS_UNAVAIL;
+ int no_more;
+ struct resolv_context *res_ctx = NULL;
+ bool do_merge = false;
+
+ /* If we do not have to look for IPv6 addresses or the canonical
+ name, use the simple, old functions, which do not support
+ IPv6 scope ids, nor retrieving the canonical name. */
+ if (req->ai_family == AF_INET
+ && (req->ai_flags & AI_CANONNAME) == 0)
{
- at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
- at->scopeid = 0;
- at->next = NULL;
-
- if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
- {
- memcpy (at->addr, addr, sizeof (at->addr));
- at->family = AF_INET;
- }
- else if (req->ai_family == AF_INET6 && (req->ai_flags & AI_V4MAPPED))
- {
- at->addr[3] = addr[0];
- at->addr[2] = htonl (0xffff);
- at->addr[1] = 0;
- at->addr[0] = 0;
- at->family = AF_INET6;
- }
- else
- {
- result = -EAI_ADDRFAMILY;
- goto free_and_return;
- }
+ int rc;
+ struct hostent th;
+ struct hostent *h;
- if (req->ai_flags & AI_CANONNAME)
+ while (1)
{
- char *canonbuf = __strdup (name);
- if (canonbuf == NULL)
+ rc = __gethostbyname2_r (name, AF_INET, &th,
+ tmpbuf->data, tmpbuf->length,
+ &h, &h_errno);
+ if (rc != ERANGE || h_errno != NETDB_INTERNAL)
+ break;
+ if (!scratch_buffer_grow (tmpbuf))
{
result = -EAI_MEMORY;
goto free_and_return;
}
- canon = canonbuf;
}
- goto process_list;
- }
-
- char *scope_delim = strchr (name, SCOPE_DELIMITER);
- int e;
-
- if (scope_delim == NULL)
- e = inet_pton (AF_INET6, name, addr);
- else
- e = __inet_pton_length (AF_INET6, name, scope_delim - name, addr);
-
- if (e > 0)
- {
- at = alloca_account (sizeof (struct gaih_addrtuple),
- alloca_used);
- at->scopeid = 0;
- at->next = NULL;
-
- if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
- {
- memcpy (at->addr, addr, sizeof (at->addr));
- at->family = AF_INET6;
- }
- else if (req->ai_family == AF_INET
- && IN6_IS_ADDR_V4MAPPED (addr))
+ if (rc == 0)
{
- at->addr[0] = addr[3];
- at->addr[1] = addr[1];
- at->addr[2] = addr[2];
- at->addr[3] = addr[3];
- at->family = AF_INET;
+ if (h != NULL)
+ {
+ /* We found data, convert it. */
+ if (!convert_hostent_to_gaih_addrtuple
+ (req, AF_INET, h, &addrmem))
+ {
+ result = -EAI_MEMORY;
+ goto free_and_return;
+ }
+ res.at = addrmem;
+ }
+ else
+ {
+ if (h_errno == NO_DATA)
+ result = -EAI_NODATA;
+ else
+ result = -EAI_NONAME;
+ goto free_and_return;
+ }
}
else
{
- result = -EAI_ADDRFAMILY;
- goto free_and_return;
- }
+ if (h_errno == NETDB_INTERNAL)
+ result = -EAI_SYSTEM;
+ else if (h_errno == TRY_AGAIN)
+ result = -EAI_AGAIN;
+ else
+ /* We made requests but they turned out no data.
+ The name is known, though. */
+ result = -EAI_NODATA;
- if (scope_delim != NULL
- && __inet6_scopeid_pton ((struct in6_addr *) at->addr,
- scope_delim + 1,
- &at->scopeid) != 0)
- {
- result = -EAI_NONAME;
goto free_and_return;
}
- if (req->ai_flags & AI_CANONNAME)
+ goto process_list;
+ }
+
+#ifdef USE_NSCD
+ if (__nss_not_use_nscd_hosts > 0
+ && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
+ __nss_not_use_nscd_hosts = 0;
+
+ if (!__nss_not_use_nscd_hosts
+ && !__nss_database_custom[NSS_DBSIDX_hosts])
+ {
+ /* Try to use nscd. */
+ struct nscd_ai_result *air = NULL;
+ int err = __nscd_getai (name, &air, &h_errno);
+ if (air != NULL)
{
- char *canonbuf = __strdup (name);
- if (canonbuf == NULL)
+ /* Transform into gaih_addrtuple list. */
+ bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
+ char *addrs = air->addrs;
+
+ addrmem = calloc (air->naddrs, sizeof (*addrmem));
+ if (addrmem == NULL)
{
result = -EAI_MEMORY;
goto free_and_return;
}
- canon = canonbuf;
- }
- goto process_list;
- }
-
- if ((req->ai_flags & AI_NUMERICHOST) == 0)
- {
- int no_data = 0;
- int no_inet6_data = 0;
- nss_action_list nip;
- enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
- enum nss_status status = NSS_STATUS_UNAVAIL;
- int no_more;
- struct resolv_context *res_ctx = NULL;
- bool do_merge = false;
-
- /* If we do not have to look for IPv6 addresses or the canonical
- name, use the simple, old functions, which do not support
- IPv6 scope ids, nor retrieving the canonical name. */
- if (req->ai_family == AF_INET
- && (req->ai_flags & AI_CANONNAME) == 0)
- {
- int rc;
- struct hostent th;
- struct hostent *h;
+ struct gaih_addrtuple *addrfree = addrmem;
+ struct gaih_addrtuple **pat = &res.at;
- while (1)
+ for (int i = 0; i < air->naddrs; ++i)
{
- rc = __gethostbyname2_r (name, AF_INET, &th,
- tmpbuf->data, tmpbuf->length,
- &h, &h_errno);
- if (rc != ERANGE || h_errno != NETDB_INTERNAL)
- break;
- if (!scratch_buffer_grow (tmpbuf))
+ socklen_t size = (air->family[i] == AF_INET
+ ? INADDRSZ : IN6ADDRSZ);
+
+ if (!((air->family[i] == AF_INET
+ && req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED) != 0)
+ || req->ai_family == AF_UNSPEC
+ || air->family[i] == req->ai_family))
{
- result = -EAI_MEMORY;
- goto free_and_return;
+ /* Skip over non-matching result. */
+ addrs += size;
+ continue;
}
- }
- if (rc == 0)
- {
- if (h != NULL)
+ if (*pat == NULL)
+ {
+ *pat = addrfree++;
+ (*pat)->scopeid = 0;
+ }
+ uint32_t *pataddr = (*pat)->addr;
+ (*pat)->next = NULL;
+ if (added_canon || air->canon == NULL)
+ (*pat)->name = NULL;
+ else if (res.canon == NULL)
{
- /* We found data, convert it. */
- if (!convert_hostent_to_gaih_addrtuple
- (req, AF_INET, h, &addrmem))
+ char *canonbuf = __strdup (air->canon);
+ if (canonbuf == NULL)
{
result = -EAI_MEMORY;
goto free_and_return;
}
- at = addrmem;
+ res.canon = (*pat)->name = canonbuf;
}
- else
+
+ if (air->family[i] == AF_INET
+ && req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED))
{
- if (h_errno == NO_DATA)
- result = -EAI_NODATA;
- else
- result = -EAI_NONAME;
- goto free_and_return;
+ (*pat)->family = AF_INET6;
+ pataddr[3] = *(uint32_t *) addrs;
+ pataddr[2] = htonl (0xffff);
+ pataddr[1] = 0;
+ pataddr[0] = 0;
+ pat = &((*pat)->next);
+ added_canon = true;
+ }
+ else if (req->ai_family == AF_UNSPEC
+ || air->family[i] == req->ai_family)
+ {
+ (*pat)->family = air->family[i];
+ memcpy (pataddr, addrs, size);
+ pat = &((*pat)->next);
+ added_canon = true;
+ if (air->family[i] == AF_INET6)
+ got_ipv6 = true;
}
+ addrs += size;
}
- else
- {
- if (h_errno == NETDB_INTERNAL)
- result = -EAI_SYSTEM;
- else if (h_errno == TRY_AGAIN)
- result = -EAI_AGAIN;
- else
- /* We made requests but they turned out no data.
- The name is known, though. */
- result = -EAI_NODATA;
- goto free_and_return;
- }
+ free (air);
goto process_list;
}
+ else if (err == 0)
+ /* The database contains a negative entry. */
+ goto free_and_return;
+ else if (__nss_not_use_nscd_hosts == 0)
+ {
+ if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
+ result = -EAI_MEMORY;
+ else if (h_errno == TRY_AGAIN)
+ result = -EAI_AGAIN;
+ else
+ result = -EAI_SYSTEM;
-#ifdef USE_NSCD
- if (__nss_not_use_nscd_hosts > 0
- && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
- __nss_not_use_nscd_hosts = 0;
+ goto free_and_return;
+ }
+ }
+#endif
+
+ no_more = !__nss_database_get (nss_database_hosts, &nip);
- if (!__nss_not_use_nscd_hosts
- && !__nss_database_custom[NSS_DBSIDX_hosts])
+ /* If we are looking for both IPv4 and IPv6 address we don't
+ want the lookup functions to automatically promote IPv4
+ addresses to IPv6 addresses, so we use the no_inet6
+ function variant. */
+ res_ctx = __resolv_context_get ();
+ if (res_ctx == NULL)
+ no_more = 1;
+
+ while (!no_more)
+ {
+ /* Always start afresh; continue should discard previous results
+ and the hosts database does not support merge. */
+ res.at = NULL;
+ free (res.canon);
+ free (addrmem);
+ res.canon = NULL;
+ addrmem = NULL;
+ got_ipv6 = false;
+
+ if (do_merge)
{
- /* Try to use nscd. */
- struct nscd_ai_result *air = NULL;
- int err = __nscd_getai (name, &air, &h_errno);
- if (air != NULL)
+ __set_h_errno (NETDB_INTERNAL);
+ __set_errno (EBUSY);
+ break;
+ }
+
+ no_data = 0;
+ nss_gethostbyname4_r *fct4 = NULL;
+
+ /* gethostbyname4_r sends out parallel A and AAAA queries and
+ is thus only suitable for PF_UNSPEC. */
+ if (req->ai_family == PF_UNSPEC)
+ fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+
+ if (fct4 != NULL)
+ {
+ while (1)
{
- /* Transform into gaih_addrtuple list. */
- bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
- char *addrs = air->addrs;
+ status = DL_CALL_FCT (fct4, (name, &res.at,
+ tmpbuf->data, tmpbuf->length,
+ &errno, &h_errno,
+ NULL));
+ if (status == NSS_STATUS_SUCCESS)
+ break;
+ /* gethostbyname4_r may write into AT, so reset it. */
+ res.at = NULL;
+ if (status != NSS_STATUS_TRYAGAIN
+ || errno != ERANGE || h_errno != NETDB_INTERNAL)
+ {
+ if (h_errno == TRY_AGAIN)
+ no_data = EAI_AGAIN;
+ else
+ no_data = h_errno == NO_DATA;
+ break;
+ }
- addrmem = calloc (air->naddrs, sizeof (*addrmem));
- if (addrmem == NULL)
+ if (!scratch_buffer_grow (tmpbuf))
{
+ __resolv_context_put (res_ctx);
result = -EAI_MEMORY;
goto free_and_return;
}
+ }
- struct gaih_addrtuple *addrfree = addrmem;
- struct gaih_addrtuple **pat = &at;
+ if (status == NSS_STATUS_SUCCESS)
+ {
+ assert (!no_data);
+ no_data = 1;
- for (int i = 0; i < air->naddrs; ++i)
+ if ((req->ai_flags & AI_CANONNAME) != 0 && res.canon == NULL)
{
- socklen_t size = (air->family[i] == AF_INET
- ? INADDRSZ : IN6ADDRSZ);
-
- if (!((air->family[i] == AF_INET
- && req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED) != 0)
- || req->ai_family == AF_UNSPEC
- || air->family[i] == req->ai_family))
+ char *canonbuf = __strdup (res.at->name);
+ if (canonbuf == NULL)
{
- /* Skip over non-matching result. */
- addrs += size;
- continue;
+ __resolv_context_put (res_ctx);
+ result = -EAI_MEMORY;
+ goto free_and_return;
}
+ res.canon = canonbuf;
+ }
- if (*pat == NULL)
- {
- *pat = addrfree++;
- (*pat)->scopeid = 0;
- }
- uint32_t *pataddr = (*pat)->addr;
- (*pat)->next = NULL;
- if (added_canon || air->canon == NULL)
- (*pat)->name = NULL;
- else if (canon == NULL)
- {
- char *canonbuf = __strdup (air->canon);
- if (canonbuf == NULL)
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- canon = (*pat)->name = canonbuf;
- }
+ struct gaih_addrtuple **pat = &res.at;
- if (air->family[i] == AF_INET
+ while (*pat != NULL)
+ {
+ if ((*pat)->family == AF_INET
&& req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED))
+ && (req->ai_flags & AI_V4MAPPED) != 0)
{
+ uint32_t *pataddr = (*pat)->addr;
(*pat)->family = AF_INET6;
- pataddr[3] = *(uint32_t *) addrs;
+ pataddr[3] = pataddr[0];
pataddr[2] = htonl (0xffff);
pataddr[1] = 0;
pataddr[0] = 0;
pat = &((*pat)->next);
- added_canon = true;
+ no_data = 0;
}
else if (req->ai_family == AF_UNSPEC
- || air->family[i] == req->ai_family)
+ || (*pat)->family == req->ai_family)
{
- (*pat)->family = air->family[i];
- memcpy (pataddr, addrs, size);
pat = &((*pat)->next);
- added_canon = true;
- if (air->family[i] == AF_INET6)
+
+ no_data = 0;
+ if (req->ai_family == AF_INET6)
got_ipv6 = true;
}
- addrs += size;
+ else
+ *pat = ((*pat)->next);
}
-
- free (air);
-
- goto process_list;
}
- else if (err == 0)
- /* The database contains a negative entry. */
- goto free_and_return;
- else if (__nss_not_use_nscd_hosts == 0)
- {
- if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
- result = -EAI_MEMORY;
- else if (h_errno == TRY_AGAIN)
- result = -EAI_AGAIN;
- else
- result = -EAI_SYSTEM;
- goto free_and_return;
- }
+ no_inet6_data = no_data;
}
-#endif
-
- no_more = !__nss_database_get (nss_database_hosts, &nip);
-
- /* If we are looking for both IPv4 and IPv6 address we don't
- want the lookup functions to automatically promote IPv4
- addresses to IPv6 addresses, so we use the no_inet6
- function variant. */
- res_ctx = __resolv_context_get ();
- if (res_ctx == NULL)
- no_more = 1;
-
- while (!no_more)
+ else
{
- /* Always start afresh; continue should discard previous results
- and the hosts database does not support merge. */
- at = NULL;
- free (canon);
- free (addrmem);
- canon = NULL;
- addrmem = NULL;
- got_ipv6 = false;
-
- if (do_merge)
+ nss_gethostbyname3_r *fct = NULL;
+ if (req->ai_flags & AI_CANONNAME)
+ /* No need to use this function if we do not look for
+ the canonical name. The function does not exist in
+ all NSS modules and therefore the lookup would
+ often fail. */
+ fct = __nss_lookup_function (nip, "gethostbyname3_r");
+ if (fct == NULL)
+ /* We are cheating here. The gethostbyname2_r
+ function does not have the same interface as
+ gethostbyname3_r but the extra arguments the
+ latter takes are added at the end. So the
+ gethostbyname2_r code will just ignore them. */
+ fct = __nss_lookup_function (nip, "gethostbyname2_r");
+
+ if (fct != NULL)
{
- __set_h_errno (NETDB_INTERNAL);
- __set_errno (EBUSY);
- break;
- }
-
- no_data = 0;
- nss_gethostbyname4_r *fct4 = NULL;
-
- /* gethostbyname4_r sends out parallel A and AAAA queries and
- is thus only suitable for PF_UNSPEC. */
- if (req->ai_family == PF_UNSPEC)
- fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+ struct gaih_addrtuple **pat = &res.at;
- if (fct4 != NULL)
- {
- while (1)
+ if (req->ai_family == AF_INET6
+ || req->ai_family == AF_UNSPEC)
{
- status = DL_CALL_FCT (fct4, (name, &at,
- tmpbuf->data, tmpbuf->length,
- &errno, &h_errno,
- NULL));
- if (status == NSS_STATUS_SUCCESS)
- break;
- /* gethostbyname4_r may write into AT, so reset it. */
- at = NULL;
- if (status != NSS_STATUS_TRYAGAIN
- || errno != ERANGE || h_errno != NETDB_INTERNAL)
- {
- if (h_errno == TRY_AGAIN)
- no_data = EAI_AGAIN;
- else
- no_data = h_errno == NO_DATA;
- break;
- }
+ gethosts (AF_INET6);
+ no_inet6_data = no_data;
+ inet6_status = status;
+ }
+ if (req->ai_family == AF_INET
+ || req->ai_family == AF_UNSPEC
+ || (req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED)
+ /* Avoid generating the mapped addresses if we
+ know we are not going to need them. */
+ && ((req->ai_flags & AI_ALL) || !got_ipv6)))
+ {
+ gethosts (AF_INET);
- if (!scratch_buffer_grow (tmpbuf))
+ if (req->ai_family == AF_INET)
{
- __resolv_context_put (res_ctx);
- result = -EAI_MEMORY;
- goto free_and_return;
+ no_inet6_data = no_data;
+ inet6_status = status;
}
}
- if (status == NSS_STATUS_SUCCESS)
+ /* If we found one address for AF_INET or AF_INET6,
+ don't continue the search. */
+ if (inet6_status == NSS_STATUS_SUCCESS
+ || status == NSS_STATUS_SUCCESS)
{
- assert (!no_data);
- no_data = 1;
-
- if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL)
+ if ((req->ai_flags & AI_CANONNAME) != 0
+ && res.canon == NULL)
{
- char *canonbuf = __strdup (at->name);
+ char *canonbuf = getcanonname (nip, res.at, name);
if (canonbuf == NULL)
{
__resolv_context_put (res_ctx);
result = -EAI_MEMORY;
goto free_and_return;
}
- canon = canonbuf;
- }
-
- struct gaih_addrtuple **pat = &at;
-
- while (*pat != NULL)
- {
- if ((*pat)->family == AF_INET
- && req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED) != 0)
- {
- uint32_t *pataddr = (*pat)->addr;
- (*pat)->family = AF_INET6;
- pataddr[3] = pataddr[0];
- pataddr[2] = htonl (0xffff);
- pataddr[1] = 0;
- pataddr[0] = 0;
- pat = &((*pat)->next);
- no_data = 0;
- }
- else if (req->ai_family == AF_UNSPEC
- || (*pat)->family == req->ai_family)
- {
- pat = &((*pat)->next);
-
- no_data = 0;
- if (req->ai_family == AF_INET6)
- got_ipv6 = true;
- }
- else
- *pat = ((*pat)->next);
- }
- }
-
- no_inet6_data = no_data;
- }
- else
- {
- nss_gethostbyname3_r *fct = NULL;
- if (req->ai_flags & AI_CANONNAME)
- /* No need to use this function if we do not look for
- the canonical name. The function does not exist in
- all NSS modules and therefore the lookup would
- often fail. */
- fct = __nss_lookup_function (nip, "gethostbyname3_r");
- if (fct == NULL)
- /* We are cheating here. The gethostbyname2_r
- function does not have the same interface as
- gethostbyname3_r but the extra arguments the
- latter takes are added at the end. So the
- gethostbyname2_r code will just ignore them. */
- fct = __nss_lookup_function (nip, "gethostbyname2_r");
-
- if (fct != NULL)
- {
- struct gaih_addrtuple **pat = &at;
-
- if (req->ai_family == AF_INET6
- || req->ai_family == AF_UNSPEC)
- {
- gethosts (AF_INET6);
- no_inet6_data = no_data;
- inet6_status = status;
- }
- if (req->ai_family == AF_INET
- || req->ai_family == AF_UNSPEC
- || (req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED)
- /* Avoid generating the mapped addresses if we
- know we are not going to need them. */
- && ((req->ai_flags & AI_ALL) || !got_ipv6)))
- {
- gethosts (AF_INET);
-
- if (req->ai_family == AF_INET)
- {
- no_inet6_data = no_data;
- inet6_status = status;
- }
- }
-
- /* If we found one address for AF_INET or AF_INET6,
- don't continue the search. */
- if (inet6_status == NSS_STATUS_SUCCESS
- || status == NSS_STATUS_SUCCESS)
- {
- if ((req->ai_flags & AI_CANONNAME) != 0
- && canon == NULL)
- {
- char *canonbuf = getcanonname (nip, at, name);
- if (canonbuf == NULL)
- {
- __resolv_context_put (res_ctx);
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- canon = canonbuf;
- }
- status = NSS_STATUS_SUCCESS;
- }
- else
- {
- /* We can have different states for AF_INET and
- AF_INET6. Try to find a useful one for both. */
- if (inet6_status == NSS_STATUS_TRYAGAIN)
- status = NSS_STATUS_TRYAGAIN;
- else if (status == NSS_STATUS_UNAVAIL
- && inet6_status != NSS_STATUS_UNAVAIL)
- status = inet6_status;
+ res.canon = canonbuf;
}
+ status = NSS_STATUS_SUCCESS;
}
else
{
- /* Could not locate any of the lookup functions.
- The NSS lookup code does not consistently set
- errno, so we need to supply our own error
- code here. The root cause could either be a
- resource allocation failure, or a missing
- service function in the DSO (so it should not
- be listed in /etc/nsswitch.conf). Assume the
- former, and return EBUSY. */
- status = NSS_STATUS_UNAVAIL;
- __set_h_errno (NETDB_INTERNAL);
- __set_errno (EBUSY);
+ /* We can have different states for AF_INET and
+ AF_INET6. Try to find a useful one for both. */
+ if (inet6_status == NSS_STATUS_TRYAGAIN)
+ status = NSS_STATUS_TRYAGAIN;
+ else if (status == NSS_STATUS_UNAVAIL
+ && inet6_status != NSS_STATUS_UNAVAIL)
+ status = inet6_status;
}
}
+ else
+ {
+ /* Could not locate any of the lookup functions.
+ The NSS lookup code does not consistently set
+ errno, so we need to supply our own error
+ code here. The root cause could either be a
+ resource allocation failure, or a missing
+ service function in the DSO (so it should not
+ be listed in /etc/nsswitch.conf). Assume the
+ former, and return EBUSY. */
+ status = NSS_STATUS_UNAVAIL;
+ __set_h_errno (NETDB_INTERNAL);
+ __set_errno (EBUSY);
+ }
+ }
- if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
- break;
+ if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
+ break;
- /* The hosts database does not support MERGE. */
- if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
- do_merge = true;
+ /* The hosts database does not support MERGE. */
+ if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
+ do_merge = true;
- nip++;
- if (nip->module == NULL)
- no_more = -1;
- }
+ nip++;
+ if (nip->module == NULL)
+ no_more = -1;
+ }
- __resolv_context_put (res_ctx);
+ __resolv_context_put (res_ctx);
- /* If we have a failure which sets errno, report it using
- EAI_SYSTEM. */
- if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
- && h_errno == NETDB_INTERNAL)
- {
- result = -EAI_SYSTEM;
- goto free_and_return;
- }
+ /* If we have a failure which sets errno, report it using
+ EAI_SYSTEM. */
+ if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+ && h_errno == NETDB_INTERNAL)
+ {
+ result = -EAI_SYSTEM;
+ goto free_and_return;
+ }
- if (no_data != 0 && no_inet6_data != 0)
- {
- /* If both requests timed out report this. */
- if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
- result = -EAI_AGAIN;
- else
- /* We made requests but they turned out no data. The name
- is known, though. */
- result = -EAI_NODATA;
+ if (no_data != 0 && no_inet6_data != 0)
+ {
+ /* If both requests timed out report this. */
+ if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
+ result = -EAI_AGAIN;
+ else
+ /* We made requests but they turned out no data. The name
+ is known, though. */
+ result = -EAI_NODATA;
- goto free_and_return;
- }
+ goto free_and_return;
}
process_list:
- if (at == NULL)
+ if (res.at == NULL)
{
result = -EAI_NONAME;
goto free_and_return;
@@ -1032,21 +1044,22 @@ gaih_inet (const char *name, const struct gaih_service *service,
else
{
struct gaih_addrtuple *atr;
- atr = at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
- memset (at, '\0', sizeof (struct gaih_addrtuple));
+ atr = res.at = alloca_account (sizeof (struct gaih_addrtuple),
+ alloca_used);
+ memset (res.at, '\0', sizeof (struct gaih_addrtuple));
if (req->ai_family == AF_UNSPEC)
{
- at->next = __alloca (sizeof (struct gaih_addrtuple));
- memset (at->next, '\0', sizeof (struct gaih_addrtuple));
+ res.at->next = __alloca (sizeof (struct gaih_addrtuple));
+ memset (res.at->next, '\0', sizeof (struct gaih_addrtuple));
}
if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
{
- at->family = AF_INET6;
+ res.at->family = AF_INET6;
if ((req->ai_flags & AI_PASSIVE) == 0)
- memcpy (at->addr, &in6addr_loopback, sizeof (struct in6_addr));
- atr = at->next;
+ memcpy (res.at->addr, &in6addr_loopback, sizeof (struct in6_addr));
+ atr = res.at->next;
}
if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
@@ -1059,10 +1072,10 @@ gaih_inet (const char *name, const struct gaih_service *service,
{
/* Set up the canonical name if we need it. */
- if ((result = process_canonname (req, orig_name, &canon)) != 0)
+ if ((result = process_canonname (req, orig_name, &res)) != 0)
goto free_and_return;
- struct gaih_addrtuple *at2 = at;
+ struct gaih_addrtuple *at2 = res.at;
size_t socklen;
sa_family_t family;
@@ -1105,8 +1118,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
ai->ai_addr = (void *) (ai + 1);
/* We only add the canonical name once. */
- ai->ai_canonname = (char *) canon;
- canon = NULL;
+ ai->ai_canonname = res.canon;
+ res.canon = NULL;
#ifdef _HAVE_SA_LEN
ai->ai_addr->sa_len = socklen;
@@ -1152,7 +1165,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
if (malloc_name)
free ((char *) name);
free (addrmem);
- free (canon);
+ free (res.canon);
return result;
}
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 06/13] gaih_inet: Split simple gethostbyname into its own function
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (4 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 05/13] gaih_inet: make numeric lookup a separate routine Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 07/13] gaih_inet: Split nscd lookup code " Siddhesh Poyarekar
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Add a free_at flag in gaih_result to indicate if res.at needs to be
freed by the caller.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit b44389cb7fa28a59804571dac09cc32ebfac03d1)
---
sysdeps/posix/getaddrinfo.c | 127 ++++++++++++++++++------------------
1 file changed, 64 insertions(+), 63 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 19bb13db59..1137c959ac 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -120,6 +120,7 @@ struct gaih_result
{
struct gaih_addrtuple *at;
char *canon;
+ bool free_at;
};
/* Values for `protoflag'. */
@@ -565,6 +566,62 @@ out:
return result;
}
+/* If possible, call the simple, old functions, which do not support IPv6 scope
+ ids, nor retrieving the canonical name. */
+
+static int
+try_simple_gethostbyname (const char *name, const struct addrinfo *req,
+ struct scratch_buffer *tmpbuf,
+ struct gaih_result *res)
+{
+ res->at = NULL;
+
+ if (req->ai_family != AF_INET || (req->ai_flags & AI_CANONNAME) != 0)
+ return 0;
+
+ int rc;
+ struct hostent th;
+ struct hostent *h;
+
+ while (1)
+ {
+ rc = __gethostbyname2_r (name, AF_INET, &th, tmpbuf->data,
+ tmpbuf->length, &h, &h_errno);
+ if (rc != ERANGE || h_errno != NETDB_INTERNAL)
+ break;
+ if (!scratch_buffer_grow (tmpbuf))
+ return -EAI_MEMORY;
+ }
+
+ if (rc == 0)
+ {
+ if (h != NULL)
+ {
+ /* We found data, convert it. RES->AT from the conversion will
+ either be an allocated block or NULL, both of which are safe to
+ pass to free (). */
+ if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, &res->at))
+ return -EAI_MEMORY;
+
+ res->free_at = true;
+ return 0;
+ }
+ if (h_errno == NO_DATA)
+ return -EAI_NODATA;
+
+ return -EAI_NONAME;
+ }
+
+ if (h_errno == NETDB_INTERNAL)
+ return -EAI_SYSTEM;
+ if (h_errno == TRY_AGAIN)
+ return -EAI_AGAIN;
+
+ /* We made requests but they turned out no data.
+ The name is known, though. */
+ return -EAI_NODATA;
+}
+
static int
gaih_inet (const char *name, const struct gaih_service *service,
const struct addrinfo *req, struct addrinfo **pai,
@@ -610,6 +667,11 @@ gaih_inet (const char *name, const struct gaih_service *service,
else if (res.at != NULL)
goto process_list;
+ if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
+
int no_data = 0;
int no_inet6_data = 0;
nss_action_list nip;
@@ -619,69 +681,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct resolv_context *res_ctx = NULL;
bool do_merge = false;
- /* If we do not have to look for IPv6 addresses or the canonical
- name, use the simple, old functions, which do not support
- IPv6 scope ids, nor retrieving the canonical name. */
- if (req->ai_family == AF_INET
- && (req->ai_flags & AI_CANONNAME) == 0)
- {
- int rc;
- struct hostent th;
- struct hostent *h;
-
- while (1)
- {
- rc = __gethostbyname2_r (name, AF_INET, &th,
- tmpbuf->data, tmpbuf->length,
- &h, &h_errno);
- if (rc != ERANGE || h_errno != NETDB_INTERNAL)
- break;
- if (!scratch_buffer_grow (tmpbuf))
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- }
-
- if (rc == 0)
- {
- if (h != NULL)
- {
- /* We found data, convert it. */
- if (!convert_hostent_to_gaih_addrtuple
- (req, AF_INET, h, &addrmem))
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- res.at = addrmem;
- }
- else
- {
- if (h_errno == NO_DATA)
- result = -EAI_NODATA;
- else
- result = -EAI_NONAME;
- goto free_and_return;
- }
- }
- else
- {
- if (h_errno == NETDB_INTERNAL)
- result = -EAI_SYSTEM;
- else if (h_errno == TRY_AGAIN)
- result = -EAI_AGAIN;
- else
- /* We made requests but they turned out no data.
- The name is known, though. */
- result = -EAI_NODATA;
-
- goto free_and_return;
- }
-
- goto process_list;
- }
-
#ifdef USE_NSCD
if (__nss_not_use_nscd_hosts > 0
&& ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
@@ -1165,6 +1164,8 @@ gaih_inet (const char *name, const struct gaih_service *service,
if (malloc_name)
free ((char *) name);
free (addrmem);
+ if (res.free_at)
+ free (res.at);
free (res.canon);
return result;
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 07/13] gaih_inet: Split nscd lookup code into its own function.
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (5 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 06/13] gaih_inet: Split simple gethostbyname into its own function Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 08/13] gaih_inet: separate nss lookup loop " Siddhesh Poyarekar
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Add a new member got_ipv6 to indicate if the results have an IPv6
result and use it instead of the local got_ipv6.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit e7e5315b7fa065a9c8bf525ca9a32f46fa4837e5)
---
sysdeps/posix/getaddrinfo.c | 248 +++++++++++++++++++-----------------
1 file changed, 134 insertions(+), 114 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 1137c959ac..01be932b3f 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -121,6 +121,7 @@ struct gaih_result
struct gaih_addrtuple *at;
char *canon;
bool free_at;
+ bool got_ipv6;
};
/* Values for `protoflag'. */
@@ -316,7 +317,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
res.canon = canonbuf; \
} \
if (_family == AF_INET6 && *pat != NULL) \
- got_ipv6 = true; \
+ res.got_ipv6 = true; \
} \
}
@@ -467,6 +468,128 @@ get_servtuples (const struct gaih_service *service, const struct addrinfo *req,
return 0;
}
+#ifdef USE_NSCD
+/* Query addresses from nscd cache, returning a non-zero value on error.
+ RES members have the lookup result; RES->AT is NULL if there were no errors
+ but also no results. */
+
+static int
+get_nscd_addresses (const char *name, const struct addrinfo *req,
+ struct gaih_result *res)
+{
+ if (__nss_not_use_nscd_hosts > 0
+ && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
+ __nss_not_use_nscd_hosts = 0;
+
+ res->at = NULL;
+
+ if (__nss_not_use_nscd_hosts || __nss_database_custom[NSS_DBSIDX_hosts])
+ return 0;
+
+ /* Try to use nscd. */
+ struct nscd_ai_result *air = NULL;
+ int err = __nscd_getai (name, &air, &h_errno);
+
+ if (__glibc_unlikely (air == NULL))
+ {
+ /* The database contains a negative entry. */
+ if (err == 0)
+ return -EAI_NONAME;
+ if (__nss_not_use_nscd_hosts == 0)
+ {
+ if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
+ return -EAI_MEMORY;
+ if (h_errno == TRY_AGAIN)
+ return -EAI_AGAIN;
+ return -EAI_SYSTEM;
+ }
+ return 0;
+ }
+
+ /* Transform into gaih_addrtuple list. */
+ int result = 0;
+ char *addrs = air->addrs;
+
+ struct gaih_addrtuple *addrfree = calloc (air->naddrs, sizeof (*addrfree));
+ struct gaih_addrtuple *at = calloc (air->naddrs, sizeof (*at));
+ if (at == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto out;
+ }
+
+ res->free_at = true;
+
+ int count = 0;
+ for (int i = 0; i < air->naddrs; ++i)
+ {
+ socklen_t size = (air->family[i] == AF_INET
+ ? INADDRSZ : IN6ADDRSZ);
+
+ if (!((air->family[i] == AF_INET
+ && req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED) != 0)
+ || req->ai_family == AF_UNSPEC
+ || air->family[i] == req->ai_family))
+ {
+ /* Skip over non-matching result. */
+ addrs += size;
+ continue;
+ }
+
+ if (air->family[i] == AF_INET && req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED))
+ {
+ at[count].family = AF_INET6;
+ at[count].addr[3] = *(uint32_t *) addrs;
+ at[count].addr[2] = htonl (0xffff);
+ }
+ else if (req->ai_family == AF_UNSPEC
+ || air->family[count] == req->ai_family)
+ {
+ at[count].family = air->family[count];
+ memcpy (at[count].addr, addrs, size);
+ if (air->family[count] == AF_INET6)
+ res->got_ipv6 = true;
+ }
+ at[count].next = at + count + 1;
+ count++;
+ addrs += size;
+ }
+
+ if ((req->ai_flags & AI_CANONNAME) && air->canon != NULL)
+ {
+ char *canonbuf = __strdup (air->canon);
+ if (canonbuf == NULL)
+ {
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ res->canon = canonbuf;
+ }
+
+ if (count == 0)
+ {
+ result = -EAI_NONAME;
+ goto out;
+ }
+
+ at[count - 1].next = NULL;
+
+ res->at = at;
+
+out:
+ free (air);
+ if (result != 0)
+ {
+ free (at);
+ res->free_at = false;
+ }
+
+ return result;
+}
+#endif
+
/* Convert numeric addresses to binary into RES. On failure, RES->AT is set to
NULL and an error code is returned. If AI_NUMERIC_HOST is not requested and
the function cannot determine a result, RES->AT is set to NULL and 0
@@ -630,7 +753,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct gaih_servtuple st[sizeof (gaih_inet_typeproto)
/ sizeof (struct gaih_typeproto)] = {0};
- bool got_ipv6 = false;
const char *orig_name = name;
/* Reserve stack memory for the scratch buffer in the getaddrinfo
@@ -672,6 +794,13 @@ gaih_inet (const char *name, const struct gaih_service *service,
else if (res.at != NULL)
goto process_list;
+#ifdef USE_NSCD
+ if ((result = get_nscd_addresses (name, req, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
+#endif
+
int no_data = 0;
int no_inet6_data = 0;
nss_action_list nip;
@@ -681,115 +810,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
struct resolv_context *res_ctx = NULL;
bool do_merge = false;
-#ifdef USE_NSCD
- if (__nss_not_use_nscd_hosts > 0
- && ++__nss_not_use_nscd_hosts > NSS_NSCD_RETRY)
- __nss_not_use_nscd_hosts = 0;
-
- if (!__nss_not_use_nscd_hosts
- && !__nss_database_custom[NSS_DBSIDX_hosts])
- {
- /* Try to use nscd. */
- struct nscd_ai_result *air = NULL;
- int err = __nscd_getai (name, &air, &h_errno);
- if (air != NULL)
- {
- /* Transform into gaih_addrtuple list. */
- bool added_canon = (req->ai_flags & AI_CANONNAME) == 0;
- char *addrs = air->addrs;
-
- addrmem = calloc (air->naddrs, sizeof (*addrmem));
- if (addrmem == NULL)
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
-
- struct gaih_addrtuple *addrfree = addrmem;
- struct gaih_addrtuple **pat = &res.at;
-
- for (int i = 0; i < air->naddrs; ++i)
- {
- socklen_t size = (air->family[i] == AF_INET
- ? INADDRSZ : IN6ADDRSZ);
-
- if (!((air->family[i] == AF_INET
- && req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED) != 0)
- || req->ai_family == AF_UNSPEC
- || air->family[i] == req->ai_family))
- {
- /* Skip over non-matching result. */
- addrs += size;
- continue;
- }
-
- if (*pat == NULL)
- {
- *pat = addrfree++;
- (*pat)->scopeid = 0;
- }
- uint32_t *pataddr = (*pat)->addr;
- (*pat)->next = NULL;
- if (added_canon || air->canon == NULL)
- (*pat)->name = NULL;
- else if (res.canon == NULL)
- {
- char *canonbuf = __strdup (air->canon);
- if (canonbuf == NULL)
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- res.canon = (*pat)->name = canonbuf;
- }
-
- if (air->family[i] == AF_INET
- && req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED))
- {
- (*pat)->family = AF_INET6;
- pataddr[3] = *(uint32_t *) addrs;
- pataddr[2] = htonl (0xffff);
- pataddr[1] = 0;
- pataddr[0] = 0;
- pat = &((*pat)->next);
- added_canon = true;
- }
- else if (req->ai_family == AF_UNSPEC
- || air->family[i] == req->ai_family)
- {
- (*pat)->family = air->family[i];
- memcpy (pataddr, addrs, size);
- pat = &((*pat)->next);
- added_canon = true;
- if (air->family[i] == AF_INET6)
- got_ipv6 = true;
- }
- addrs += size;
- }
-
- free (air);
-
- goto process_list;
- }
- else if (err == 0)
- /* The database contains a negative entry. */
- goto free_and_return;
- else if (__nss_not_use_nscd_hosts == 0)
- {
- if (h_errno == NETDB_INTERNAL && errno == ENOMEM)
- result = -EAI_MEMORY;
- else if (h_errno == TRY_AGAIN)
- result = -EAI_AGAIN;
- else
- result = -EAI_SYSTEM;
-
- goto free_and_return;
- }
- }
-#endif
-
no_more = !__nss_database_get (nss_database_hosts, &nip);
/* If we are looking for both IPv4 and IPv6 address we don't
@@ -897,7 +917,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
no_data = 0;
if (req->ai_family == AF_INET6)
- got_ipv6 = true;
+ res.got_ipv6 = true;
}
else
*pat = ((*pat)->next);
@@ -940,7 +960,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
&& (req->ai_flags & AI_V4MAPPED)
/* Avoid generating the mapped addresses if we
know we are not going to need them. */
- && ((req->ai_flags & AI_ALL) || !got_ipv6)))
+ && ((req->ai_flags & AI_ALL) || !res.got_ipv6)))
{
gethosts (AF_INET);
@@ -1091,7 +1111,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
/* If we looked up IPv4 mapped address discard them here if
the caller isn't interested in all address and we have
found at least one IPv6 address. */
- if (got_ipv6
+ if (res.got_ipv6
&& (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
&& IN6_IS_ADDR_V4MAPPED (at2->addr))
goto ignore;
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 08/13] gaih_inet: separate nss lookup loop into its own function
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (6 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 07/13] gaih_inet: Split nscd lookup code " Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 09/13] gaih_inet: make gethosts into a function Siddhesh Poyarekar
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 906cecbe0889e601c91d9aba738049c73ebe4dd2)
---
sysdeps/posix/getaddrinfo.c | 563 ++++++++++++++++++------------------
1 file changed, 286 insertions(+), 277 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 01be932b3f..f70ce2c76b 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -159,6 +159,14 @@ static const struct addrinfo default_hints =
.ai_next = NULL
};
+static void
+gaih_result_reset (struct gaih_result *res)
+{
+ if (res->free_at)
+ free (res->at);
+ free (res->canon);
+ memset (res, 0, sizeof (*res));
+}
static int
gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
@@ -197,13 +205,10 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
/* Convert struct hostent to a list of struct gaih_addrtuple objects. h_name
is not copied, and the struct hostent object must not be deallocated
- prematurely. The new addresses are appended to the tuple array in
- RESULT. */
+ prematurely. The new addresses are appended to the tuple array in RES. */
static bool
-convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
- int family,
- struct hostent *h,
- struct gaih_addrtuple **result)
+convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
+ struct hostent *h, struct gaih_result *res)
{
/* Count the number of addresses in h->h_addr_list. */
size_t count = 0;
@@ -215,7 +220,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
return true;
- struct gaih_addrtuple *array = *result;
+ struct gaih_addrtuple *array = res->at;
size_t old = 0;
while (array != NULL)
@@ -224,12 +229,14 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
array = array->next;
}
- array = realloc (*result, (old + count) * sizeof (*array));
+ array = realloc (res->at, (old + count) * sizeof (*array));
if (array == NULL)
return false;
- *result = array;
+ res->got_ipv6 = family == AF_INET6;
+ res->at = array;
+ res->free_at = true;
/* Update the next pointers on reallocation. */
for (size_t i = 0; i < old; i++)
@@ -278,7 +285,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
{ \
__resolv_context_put (res_ctx); \
result = -EAI_MEMORY; \
- goto free_and_return; \
+ goto out; \
} \
} \
if (status == NSS_STATUS_NOTFOUND \
@@ -288,7 +295,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
{ \
__resolv_context_put (res_ctx); \
result = -EAI_SYSTEM; \
- goto free_and_return; \
+ goto out; \
} \
if (h_errno == TRY_AGAIN) \
no_data = EAI_AGAIN; \
@@ -297,27 +304,24 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
} \
else if (status == NSS_STATUS_SUCCESS) \
{ \
- if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, &addrmem)) \
+ if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, res)) \
{ \
__resolv_context_put (res_ctx); \
result = -EAI_SYSTEM; \
- goto free_and_return; \
+ goto out; \
} \
- *pat = addrmem; \
\
- if (localcanon != NULL && res.canon == NULL) \
+ if (localcanon != NULL && res->canon == NULL) \
{ \
char *canonbuf = __strdup (localcanon); \
if (canonbuf == NULL) \
{ \
__resolv_context_put (res_ctx); \
result = -EAI_SYSTEM; \
- goto free_and_return; \
+ goto out; \
} \
- res.canon = canonbuf; \
+ res->canon = canonbuf; \
} \
- if (_family == AF_INET6 && *pat != NULL) \
- res.got_ipv6 = true; \
} \
}
@@ -590,6 +594,260 @@ out:
}
#endif
+static int
+get_nss_addresses (const char *name, const struct addrinfo *req,
+ struct scratch_buffer *tmpbuf, struct gaih_result *res)
+{
+ int no_data = 0;
+ int no_inet6_data = 0;
+ nss_action_list nip;
+ enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
+ enum nss_status status = NSS_STATUS_UNAVAIL;
+ int no_more;
+ struct resolv_context *res_ctx = NULL;
+ bool do_merge = false;
+ int result = 0;
+
+ no_more = !__nss_database_get (nss_database_hosts, &nip);
+
+ /* If we are looking for both IPv4 and IPv6 address we don't
+ want the lookup functions to automatically promote IPv4
+ addresses to IPv6 addresses, so we use the no_inet6
+ function variant. */
+ res_ctx = __resolv_context_get ();
+ if (res_ctx == NULL)
+ no_more = 1;
+
+ while (!no_more)
+ {
+ /* Always start afresh; continue should discard previous results
+ and the hosts database does not support merge. */
+ gaih_result_reset (res);
+
+ if (do_merge)
+ {
+ __set_h_errno (NETDB_INTERNAL);
+ __set_errno (EBUSY);
+ break;
+ }
+
+ no_data = 0;
+ nss_gethostbyname4_r *fct4 = NULL;
+
+ /* gethostbyname4_r sends out parallel A and AAAA queries and
+ is thus only suitable for PF_UNSPEC. */
+ if (req->ai_family == PF_UNSPEC)
+ fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
+
+ if (fct4 != NULL)
+ {
+ while (1)
+ {
+ status = DL_CALL_FCT (fct4, (name, &res->at,
+ tmpbuf->data, tmpbuf->length,
+ &errno, &h_errno,
+ NULL));
+ if (status == NSS_STATUS_SUCCESS)
+ break;
+ /* gethostbyname4_r may write into AT, so reset it. */
+ res->at = NULL;
+ if (status != NSS_STATUS_TRYAGAIN
+ || errno != ERANGE || h_errno != NETDB_INTERNAL)
+ {
+ if (h_errno == TRY_AGAIN)
+ no_data = EAI_AGAIN;
+ else
+ no_data = h_errno == NO_DATA;
+ break;
+ }
+
+ if (!scratch_buffer_grow (tmpbuf))
+ {
+ __resolv_context_put (res_ctx);
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ }
+
+ if (status == NSS_STATUS_SUCCESS)
+ {
+ assert (!no_data);
+ no_data = 1;
+
+ if ((req->ai_flags & AI_CANONNAME) != 0 && res->canon == NULL)
+ {
+ char *canonbuf = __strdup (res->at->name);
+ if (canonbuf == NULL)
+ {
+ __resolv_context_put (res_ctx);
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ res->canon = canonbuf;
+ }
+
+ struct gaih_addrtuple **pat = &res->at;
+
+ while (*pat != NULL)
+ {
+ if ((*pat)->family == AF_INET
+ && req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED) != 0)
+ {
+ uint32_t *pataddr = (*pat)->addr;
+ (*pat)->family = AF_INET6;
+ pataddr[3] = pataddr[0];
+ pataddr[2] = htonl (0xffff);
+ pataddr[1] = 0;
+ pataddr[0] = 0;
+ pat = &((*pat)->next);
+ no_data = 0;
+ }
+ else if (req->ai_family == AF_UNSPEC
+ || (*pat)->family == req->ai_family)
+ {
+ pat = &((*pat)->next);
+
+ no_data = 0;
+ if (req->ai_family == AF_INET6)
+ res->got_ipv6 = true;
+ }
+ else
+ *pat = ((*pat)->next);
+ }
+ }
+
+ no_inet6_data = no_data;
+ }
+ else
+ {
+ nss_gethostbyname3_r *fct = NULL;
+ if (req->ai_flags & AI_CANONNAME)
+ /* No need to use this function if we do not look for
+ the canonical name. The function does not exist in
+ all NSS modules and therefore the lookup would
+ often fail. */
+ fct = __nss_lookup_function (nip, "gethostbyname3_r");
+ if (fct == NULL)
+ /* We are cheating here. The gethostbyname2_r
+ function does not have the same interface as
+ gethostbyname3_r but the extra arguments the
+ latter takes are added at the end. So the
+ gethostbyname2_r code will just ignore them. */
+ fct = __nss_lookup_function (nip, "gethostbyname2_r");
+
+ if (fct != NULL)
+ {
+ if (req->ai_family == AF_INET6
+ || req->ai_family == AF_UNSPEC)
+ {
+ gethosts (AF_INET6);
+ no_inet6_data = no_data;
+ inet6_status = status;
+ }
+ if (req->ai_family == AF_INET
+ || req->ai_family == AF_UNSPEC
+ || (req->ai_family == AF_INET6
+ && (req->ai_flags & AI_V4MAPPED)
+ /* Avoid generating the mapped addresses if we
+ know we are not going to need them. */
+ && ((req->ai_flags & AI_ALL) || !res->got_ipv6)))
+ {
+ gethosts (AF_INET);
+
+ if (req->ai_family == AF_INET)
+ {
+ no_inet6_data = no_data;
+ inet6_status = status;
+ }
+ }
+
+ /* If we found one address for AF_INET or AF_INET6,
+ don't continue the search. */
+ if (inet6_status == NSS_STATUS_SUCCESS
+ || status == NSS_STATUS_SUCCESS)
+ {
+ if ((req->ai_flags & AI_CANONNAME) != 0
+ && res->canon == NULL)
+ {
+ char *canonbuf = getcanonname (nip, res->at, name);
+ if (canonbuf == NULL)
+ {
+ __resolv_context_put (res_ctx);
+ result = -EAI_MEMORY;
+ goto out;
+ }
+ res->canon = canonbuf;
+ }
+ status = NSS_STATUS_SUCCESS;
+ }
+ else
+ {
+ /* We can have different states for AF_INET and
+ AF_INET6. Try to find a useful one for both. */
+ if (inet6_status == NSS_STATUS_TRYAGAIN)
+ status = NSS_STATUS_TRYAGAIN;
+ else if (status == NSS_STATUS_UNAVAIL
+ && inet6_status != NSS_STATUS_UNAVAIL)
+ status = inet6_status;
+ }
+ }
+ else
+ {
+ /* Could not locate any of the lookup functions.
+ The NSS lookup code does not consistently set
+ errno, so we need to supply our own error
+ code here. The root cause could either be a
+ resource allocation failure, or a missing
+ service function in the DSO (so it should not
+ be listed in /etc/nsswitch.conf). Assume the
+ former, and return EBUSY. */
+ status = NSS_STATUS_UNAVAIL;
+ __set_h_errno (NETDB_INTERNAL);
+ __set_errno (EBUSY);
+ }
+ }
+
+ if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
+ break;
+
+ /* The hosts database does not support MERGE. */
+ if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
+ do_merge = true;
+
+ nip++;
+ if (nip->module == NULL)
+ no_more = -1;
+ }
+
+ __resolv_context_put (res_ctx);
+
+ /* If we have a failure which sets errno, report it using
+ EAI_SYSTEM. */
+ if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+ && h_errno == NETDB_INTERNAL)
+ {
+ result = -EAI_SYSTEM;
+ goto out;
+ }
+
+ if (no_data != 0 && no_inet6_data != 0)
+ {
+ /* If both requests timed out report this. */
+ if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
+ result = -EAI_AGAIN;
+ else
+ /* We made requests but they turned out no data. The name
+ is known, though. */
+ result = -EAI_NODATA;
+ }
+
+out:
+ if (result != 0)
+ gaih_result_reset (res);
+ return result;
+}
+
/* Convert numeric addresses to binary into RES. On failure, RES->AT is set to
NULL and an error code is returned. If AI_NUMERIC_HOST is not requested and
the function cannot determine a result, RES->AT is set to NULL and 0
@@ -723,7 +981,7 @@ try_simple_gethostbyname (const char *name, const struct addrinfo *req,
/* We found data, convert it. RES->AT from the conversion will
either be an allocated block or NULL, both of which are safe to
pass to free (). */
- if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, &res->at))
+ if (!convert_hostent_to_gaih_addrtuple (req, AF_INET, h, res))
return -EAI_MEMORY;
res->free_at = true;
@@ -801,264 +1059,14 @@ gaih_inet (const char *name, const struct gaih_service *service,
goto process_list;
#endif
- int no_data = 0;
- int no_inet6_data = 0;
- nss_action_list nip;
- enum nss_status inet6_status = NSS_STATUS_UNAVAIL;
- enum nss_status status = NSS_STATUS_UNAVAIL;
- int no_more;
- struct resolv_context *res_ctx = NULL;
- bool do_merge = false;
-
- no_more = !__nss_database_get (nss_database_hosts, &nip);
-
- /* If we are looking for both IPv4 and IPv6 address we don't
- want the lookup functions to automatically promote IPv4
- addresses to IPv6 addresses, so we use the no_inet6
- function variant. */
- res_ctx = __resolv_context_get ();
- if (res_ctx == NULL)
- no_more = 1;
-
- while (!no_more)
- {
- /* Always start afresh; continue should discard previous results
- and the hosts database does not support merge. */
- res.at = NULL;
- free (res.canon);
- free (addrmem);
- res.canon = NULL;
- addrmem = NULL;
- got_ipv6 = false;
-
- if (do_merge)
- {
- __set_h_errno (NETDB_INTERNAL);
- __set_errno (EBUSY);
- break;
- }
-
- no_data = 0;
- nss_gethostbyname4_r *fct4 = NULL;
-
- /* gethostbyname4_r sends out parallel A and AAAA queries and
- is thus only suitable for PF_UNSPEC. */
- if (req->ai_family == PF_UNSPEC)
- fct4 = __nss_lookup_function (nip, "gethostbyname4_r");
-
- if (fct4 != NULL)
- {
- while (1)
- {
- status = DL_CALL_FCT (fct4, (name, &res.at,
- tmpbuf->data, tmpbuf->length,
- &errno, &h_errno,
- NULL));
- if (status == NSS_STATUS_SUCCESS)
- break;
- /* gethostbyname4_r may write into AT, so reset it. */
- res.at = NULL;
- if (status != NSS_STATUS_TRYAGAIN
- || errno != ERANGE || h_errno != NETDB_INTERNAL)
- {
- if (h_errno == TRY_AGAIN)
- no_data = EAI_AGAIN;
- else
- no_data = h_errno == NO_DATA;
- break;
- }
-
- if (!scratch_buffer_grow (tmpbuf))
- {
- __resolv_context_put (res_ctx);
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- }
-
- if (status == NSS_STATUS_SUCCESS)
- {
- assert (!no_data);
- no_data = 1;
-
- if ((req->ai_flags & AI_CANONNAME) != 0 && res.canon == NULL)
- {
- char *canonbuf = __strdup (res.at->name);
- if (canonbuf == NULL)
- {
- __resolv_context_put (res_ctx);
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- res.canon = canonbuf;
- }
-
- struct gaih_addrtuple **pat = &res.at;
-
- while (*pat != NULL)
- {
- if ((*pat)->family == AF_INET
- && req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED) != 0)
- {
- uint32_t *pataddr = (*pat)->addr;
- (*pat)->family = AF_INET6;
- pataddr[3] = pataddr[0];
- pataddr[2] = htonl (0xffff);
- pataddr[1] = 0;
- pataddr[0] = 0;
- pat = &((*pat)->next);
- no_data = 0;
- }
- else if (req->ai_family == AF_UNSPEC
- || (*pat)->family == req->ai_family)
- {
- pat = &((*pat)->next);
-
- no_data = 0;
- if (req->ai_family == AF_INET6)
- res.got_ipv6 = true;
- }
- else
- *pat = ((*pat)->next);
- }
- }
-
- no_inet6_data = no_data;
- }
- else
- {
- nss_gethostbyname3_r *fct = NULL;
- if (req->ai_flags & AI_CANONNAME)
- /* No need to use this function if we do not look for
- the canonical name. The function does not exist in
- all NSS modules and therefore the lookup would
- often fail. */
- fct = __nss_lookup_function (nip, "gethostbyname3_r");
- if (fct == NULL)
- /* We are cheating here. The gethostbyname2_r
- function does not have the same interface as
- gethostbyname3_r but the extra arguments the
- latter takes are added at the end. So the
- gethostbyname2_r code will just ignore them. */
- fct = __nss_lookup_function (nip, "gethostbyname2_r");
-
- if (fct != NULL)
- {
- struct gaih_addrtuple **pat = &res.at;
-
- if (req->ai_family == AF_INET6
- || req->ai_family == AF_UNSPEC)
- {
- gethosts (AF_INET6);
- no_inet6_data = no_data;
- inet6_status = status;
- }
- if (req->ai_family == AF_INET
- || req->ai_family == AF_UNSPEC
- || (req->ai_family == AF_INET6
- && (req->ai_flags & AI_V4MAPPED)
- /* Avoid generating the mapped addresses if we
- know we are not going to need them. */
- && ((req->ai_flags & AI_ALL) || !res.got_ipv6)))
- {
- gethosts (AF_INET);
-
- if (req->ai_family == AF_INET)
- {
- no_inet6_data = no_data;
- inet6_status = status;
- }
- }
-
- /* If we found one address for AF_INET or AF_INET6,
- don't continue the search. */
- if (inet6_status == NSS_STATUS_SUCCESS
- || status == NSS_STATUS_SUCCESS)
- {
- if ((req->ai_flags & AI_CANONNAME) != 0
- && res.canon == NULL)
- {
- char *canonbuf = getcanonname (nip, res.at, name);
- if (canonbuf == NULL)
- {
- __resolv_context_put (res_ctx);
- result = -EAI_MEMORY;
- goto free_and_return;
- }
- res.canon = canonbuf;
- }
- status = NSS_STATUS_SUCCESS;
- }
- else
- {
- /* We can have different states for AF_INET and
- AF_INET6. Try to find a useful one for both. */
- if (inet6_status == NSS_STATUS_TRYAGAIN)
- status = NSS_STATUS_TRYAGAIN;
- else if (status == NSS_STATUS_UNAVAIL
- && inet6_status != NSS_STATUS_UNAVAIL)
- status = inet6_status;
- }
- }
- else
- {
- /* Could not locate any of the lookup functions.
- The NSS lookup code does not consistently set
- errno, so we need to supply our own error
- code here. The root cause could either be a
- resource allocation failure, or a missing
- service function in the DSO (so it should not
- be listed in /etc/nsswitch.conf). Assume the
- former, and return EBUSY. */
- status = NSS_STATUS_UNAVAIL;
- __set_h_errno (NETDB_INTERNAL);
- __set_errno (EBUSY);
- }
- }
-
- if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
- break;
-
- /* The hosts database does not support MERGE. */
- if (nss_next_action (nip, status) == NSS_ACTION_MERGE)
- do_merge = true;
-
- nip++;
- if (nip->module == NULL)
- no_more = -1;
- }
-
- __resolv_context_put (res_ctx);
-
- /* If we have a failure which sets errno, report it using
- EAI_SYSTEM. */
- if ((status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
- && h_errno == NETDB_INTERNAL)
- {
- result = -EAI_SYSTEM;
- goto free_and_return;
- }
-
- if (no_data != 0 && no_inet6_data != 0)
- {
- /* If both requests timed out report this. */
- if (no_data == EAI_AGAIN && no_inet6_data == EAI_AGAIN)
- result = -EAI_AGAIN;
- else
- /* We made requests but they turned out no data. The name
- is known, though. */
- result = -EAI_NODATA;
-
- goto free_and_return;
- }
+ if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
- process_list:
- if (res.at == NULL)
- {
- result = -EAI_NONAME;
- goto free_and_return;
- }
+ /* None of the lookups worked, so name not found. */
+ result = -EAI_NONAME;
+ goto free_and_return;
}
else
{
@@ -1089,6 +1097,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
}
}
+process_list:
{
/* Set up the canonical name if we need it. */
if ((result = process_canonname (req, orig_name, &res)) != 0)
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 09/13] gaih_inet: make gethosts into a function
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (7 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 08/13] gaih_inet: separate nss lookup loop " Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 10/13] gaih_inet: split loopback lookup into its own function Siddhesh Poyarekar
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
The macro is quite a pain to debug, so make gethosts into a function to
make it easier to maintain.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit cfa3bd48cb19a70e4367a9978dbba09d9df27a72)
---
sysdeps/posix/getaddrinfo.c | 117 ++++++++++++++++++------------------
1 file changed, 59 insertions(+), 58 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index f70ce2c76b..bc385dd322 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -268,63 +268,54 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
return true;
}
-#define gethosts(_family) \
- { \
- struct hostent th; \
- char *localcanon = NULL; \
- no_data = 0; \
- while (1) \
- { \
- status = DL_CALL_FCT (fct, (name, _family, &th, \
- tmpbuf->data, tmpbuf->length, \
- &errno, &h_errno, NULL, &localcanon)); \
- if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL \
- || errno != ERANGE) \
- break; \
- if (!scratch_buffer_grow (tmpbuf)) \
- { \
- __resolv_context_put (res_ctx); \
- result = -EAI_MEMORY; \
- goto out; \
- } \
- } \
- if (status == NSS_STATUS_NOTFOUND \
- || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL) \
- { \
- if (h_errno == NETDB_INTERNAL) \
- { \
- __resolv_context_put (res_ctx); \
- result = -EAI_SYSTEM; \
- goto out; \
- } \
- if (h_errno == TRY_AGAIN) \
- no_data = EAI_AGAIN; \
- else \
- no_data = h_errno == NO_DATA; \
- } \
- else if (status == NSS_STATUS_SUCCESS) \
- { \
- if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, res)) \
- { \
- __resolv_context_put (res_ctx); \
- result = -EAI_SYSTEM; \
- goto out; \
- } \
- \
- if (localcanon != NULL && res->canon == NULL) \
- { \
- char *canonbuf = __strdup (localcanon); \
- if (canonbuf == NULL) \
- { \
- __resolv_context_put (res_ctx); \
- result = -EAI_SYSTEM; \
- goto out; \
- } \
- res->canon = canonbuf; \
- } \
- } \
- }
+static int
+gethosts (nss_gethostbyname3_r fct, int family, const char *name,
+ const struct addrinfo *req, struct scratch_buffer *tmpbuf,
+ struct gaih_result *res, enum nss_status *statusp, int *no_datap)
+{
+ struct hostent th;
+ char *localcanon = NULL;
+ enum nss_status status;
+
+ *no_datap = 0;
+ while (1)
+ {
+ *statusp = status = DL_CALL_FCT (fct, (name, family, &th,
+ tmpbuf->data, tmpbuf->length,
+ &errno, &h_errno, NULL,
+ &localcanon));
+ if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL
+ || errno != ERANGE)
+ break;
+ if (!scratch_buffer_grow (tmpbuf))
+ return -EAI_MEMORY;
+ }
+ if (status == NSS_STATUS_NOTFOUND
+ || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
+ {
+ if (h_errno == NETDB_INTERNAL)
+ return -EAI_SYSTEM;
+ if (h_errno == TRY_AGAIN)
+ *no_datap = EAI_AGAIN;
+ else
+ *no_datap = h_errno == NO_DATA;
+ }
+ else if (status == NSS_STATUS_SUCCESS)
+ {
+ if (!convert_hostent_to_gaih_addrtuple (req, family, &th, res))
+ return -EAI_SYSTEM;
+
+ if (localcanon != NULL && res->canon == NULL)
+ {
+ char *canonbuf = __strdup (localcanon);
+ if (canonbuf == NULL)
+ return -EAI_SYSTEM;
+ res->canon = canonbuf;
+ }
+ }
+ return 0;
+}
/* This function is called if a canonical name is requested, but if
the service function did not provide it. It tries to obtain the
@@ -741,7 +732,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
if (req->ai_family == AF_INET6
|| req->ai_family == AF_UNSPEC)
{
- gethosts (AF_INET6);
+ if ((result = gethosts (fct, AF_INET6, name, req, tmpbuf,
+ res, &status, &no_data)) != 0)
+ {
+ __resolv_context_put (res_ctx);
+ goto out;
+ }
no_inet6_data = no_data;
inet6_status = status;
}
@@ -753,7 +749,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
know we are not going to need them. */
&& ((req->ai_flags & AI_ALL) || !res->got_ipv6)))
{
- gethosts (AF_INET);
+ if ((result = gethosts (fct, AF_INET, name, req, tmpbuf,
+ res, &status, &no_data)) != 0)
+ {
+ __resolv_context_put (res_ctx);
+ goto out;
+ }
if (req->ai_family == AF_INET)
{
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 10/13] gaih_inet: split loopback lookup into its own function
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (8 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 09/13] gaih_inet: make gethosts into a function Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 11/13] gaih_inet: Split result generation " Siddhesh Poyarekar
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Flatten the condition nesting and replace the alloca for RET.AT/ATR with
a single array LOCAL_AT[2]. This gets rid of alloca and alloca
accounting.
`git diff -b` is probably the best way to view this change since much of
the diff is whitespace changes.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit 657472b2a50f67b12e5bbe5827582c9c2bb82dc3)
---
sysdeps/posix/getaddrinfo.c | 127 ++++++++++++++++++------------------
1 file changed, 62 insertions(+), 65 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index bc385dd322..47c41d332d 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -1004,6 +1004,32 @@ try_simple_gethostbyname (const char *name, const struct addrinfo *req,
return -EAI_NODATA;
}
+/* Add local address information into RES. RES->AT is assumed to have enough
+ space for two tuples and is zeroed out. */
+
+static void
+get_local_addresses (const struct addrinfo *req, struct gaih_result *res)
+{
+ struct gaih_addrtuple *atr = res->at;
+ if (req->ai_family == AF_UNSPEC)
+ res->at->next = res->at + 1;
+
+ if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
+ {
+ res->at->family = AF_INET6;
+ if ((req->ai_flags & AI_PASSIVE) == 0)
+ memcpy (res->at->addr, &in6addr_loopback, sizeof (struct in6_addr));
+ atr = res->at->next;
+ }
+
+ if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
+ {
+ atr->family = AF_INET;
+ if ((req->ai_flags & AI_PASSIVE) == 0)
+ atr->addr[0] = htonl (INADDR_LOOPBACK);
+ }
+}
+
static int
gaih_inet (const char *name, const struct gaih_service *service,
const struct addrinfo *req, struct addrinfo **pai,
@@ -1014,10 +1040,6 @@ gaih_inet (const char *name, const struct gaih_service *service,
const char *orig_name = name;
- /* Reserve stack memory for the scratch buffer in the getaddrinfo
- function. */
- size_t alloca_used = sizeof (struct scratch_buffer);
-
int rc;
if ((rc = get_servtuples (service, req, st, tmpbuf)) != 0)
return rc;
@@ -1027,76 +1049,51 @@ gaih_inet (const char *name, const struct gaih_service *service,
int result = 0;
struct gaih_result res = {0};
- if (name != NULL)
+ struct gaih_addrtuple local_at[2] = {0};
+
+ res.at = local_at;
+
+ if (__glibc_unlikely (name == NULL))
{
- if (req->ai_flags & AI_IDN)
- {
- char *out;
- result = __idna_to_dns_encoding (name, &out);
- if (result != 0)
- return -result;
- name = out;
- malloc_name = true;
- }
+ get_local_addresses (req, &res);
+ goto process_list;
+ }
- res.at = alloca_account (sizeof (struct gaih_addrtuple), alloca_used);
- res.at->scopeid = 0;
- res.at->next = NULL;
+ if (req->ai_flags & AI_IDN)
+ {
+ char *out;
+ result = __idna_to_dns_encoding (name, &out);
+ if (result != 0)
+ return -result;
+ name = out;
+ malloc_name = true;
+ }
- if ((result = text_to_binary_address (name, req, &res)) != 0)
- goto free_and_return;
- else if (res.at != NULL)
- goto process_list;
+ if ((result = text_to_binary_address (name, req, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
- if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
- goto free_and_return;
- else if (res.at != NULL)
- goto process_list;
+ if ((result = try_simple_gethostbyname (name, req, tmpbuf, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
#ifdef USE_NSCD
- if ((result = get_nscd_addresses (name, req, &res)) != 0)
- goto free_and_return;
- else if (res.at != NULL)
- goto process_list;
+ if ((result = get_nscd_addresses (name, req, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
#endif
- if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
- goto free_and_return;
- else if (res.at != NULL)
- goto process_list;
-
- /* None of the lookups worked, so name not found. */
- result = -EAI_NONAME;
- goto free_and_return;
- }
- else
- {
- struct gaih_addrtuple *atr;
- atr = res.at = alloca_account (sizeof (struct gaih_addrtuple),
- alloca_used);
- memset (res.at, '\0', sizeof (struct gaih_addrtuple));
-
- if (req->ai_family == AF_UNSPEC)
- {
- res.at->next = __alloca (sizeof (struct gaih_addrtuple));
- memset (res.at->next, '\0', sizeof (struct gaih_addrtuple));
- }
-
- if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET6)
- {
- res.at->family = AF_INET6;
- if ((req->ai_flags & AI_PASSIVE) == 0)
- memcpy (res.at->addr, &in6addr_loopback, sizeof (struct in6_addr));
- atr = res.at->next;
- }
+ if ((result = get_nss_addresses (name, req, tmpbuf, &res)) != 0)
+ goto free_and_return;
+ else if (res.at != NULL)
+ goto process_list;
- if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
- {
- atr->family = AF_INET;
- if ((req->ai_flags & AI_PASSIVE) == 0)
- atr->addr[0] = htonl (INADDR_LOOPBACK);
- }
- }
+ /* None of the lookups worked, so name not found. */
+ result = -EAI_NONAME;
+ goto free_and_return;
process_list:
{
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 11/13] gaih_inet: Split result generation into its own function
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (9 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 10/13] gaih_inet: split loopback lookup into its own function Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 12/13] gethosts: Return EAI_MEMORY on allocation failure Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 13/13] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) Siddhesh Poyarekar
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
Simplify the loop a wee bit and clean up variable names too.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit ac4653ef503d1e87893d1a6714748a1cdf4bf7ad)
---
sysdeps/posix/getaddrinfo.c | 176 ++++++++++++++++++------------------
1 file changed, 86 insertions(+), 90 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 47c41d332d..f5d4a5cfd9 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -1030,6 +1030,87 @@ get_local_addresses (const struct addrinfo *req, struct gaih_result *res)
}
}
+/* Generate results in PAI and its count in NADDRS. Return 0 on success or an
+ error code on failure. */
+
+static int
+generate_addrinfo (const struct addrinfo *req, struct gaih_result *res,
+ const struct gaih_servtuple *st, struct addrinfo **pai,
+ unsigned int *naddrs)
+{
+ size_t socklen;
+ sa_family_t family;
+
+ /* Buffer is the size of an unformatted IPv6 address in printable format. */
+ for (struct gaih_addrtuple *at = res->at; at != NULL; at = at->next)
+ {
+ family = at->family;
+ if (family == AF_INET6)
+ {
+ socklen = sizeof (struct sockaddr_in6);
+
+ /* If we looked up IPv4 mapped address discard them here if
+ the caller isn't interested in all address and we have
+ found at least one IPv6 address. */
+ if (res->got_ipv6
+ && (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
+ && IN6_IS_ADDR_V4MAPPED (at->addr))
+ continue;
+ }
+ else
+ socklen = sizeof (struct sockaddr_in);
+
+ for (int i = 0; st[i].set; i++)
+ {
+ struct addrinfo *ai;
+ ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
+ if (ai == NULL)
+ return -EAI_MEMORY;
+
+ ai->ai_flags = req->ai_flags;
+ ai->ai_family = family;
+ ai->ai_socktype = st[i].socktype;
+ ai->ai_protocol = st[i].protocol;
+ ai->ai_addrlen = socklen;
+ ai->ai_addr = (void *) (ai + 1);
+
+ /* We only add the canonical name once. */
+ ai->ai_canonname = res->canon;
+ res->canon = NULL;
+
+#ifdef _HAVE_SA_LEN
+ ai->ai_addr->sa_len = socklen;
+#endif /* _HAVE_SA_LEN */
+ ai->ai_addr->sa_family = family;
+
+ /* In case of an allocation error the list must be NULL
+ terminated. */
+ ai->ai_next = NULL;
+
+ if (family == AF_INET6)
+ {
+ struct sockaddr_in6 *sin6p = (struct sockaddr_in6 *) ai->ai_addr;
+ sin6p->sin6_port = st[i].port;
+ sin6p->sin6_flowinfo = 0;
+ memcpy (&sin6p->sin6_addr, at->addr, sizeof (struct in6_addr));
+ sin6p->sin6_scope_id = at->scopeid;
+ }
+ else
+ {
+ struct sockaddr_in *sinp = (struct sockaddr_in *) ai->ai_addr;
+ sinp->sin_port = st[i].port;
+ memcpy (&sinp->sin_addr, at->addr, sizeof (struct in_addr));
+ memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
+ }
+
+ pai = &(ai->ai_next);
+ }
+
+ ++*naddrs;
+ }
+ return 0;
+}
+
static int
gaih_inet (const char *name, const struct gaih_service *service,
const struct addrinfo *req, struct addrinfo **pai,
@@ -1096,98 +1177,13 @@ gaih_inet (const char *name, const struct gaih_service *service,
goto free_and_return;
process_list:
- {
- /* Set up the canonical name if we need it. */
- if ((result = process_canonname (req, orig_name, &res)) != 0)
- goto free_and_return;
-
- struct gaih_addrtuple *at2 = res.at;
- size_t socklen;
- sa_family_t family;
-
- /*
- buffer is the size of an unformatted IPv6 address in printable format.
- */
- while (at2 != NULL)
- {
- family = at2->family;
- if (family == AF_INET6)
- {
- socklen = sizeof (struct sockaddr_in6);
-
- /* If we looked up IPv4 mapped address discard them here if
- the caller isn't interested in all address and we have
- found at least one IPv6 address. */
- if (res.got_ipv6
- && (req->ai_flags & (AI_V4MAPPED|AI_ALL)) == AI_V4MAPPED
- && IN6_IS_ADDR_V4MAPPED (at2->addr))
- goto ignore;
- }
- else
- socklen = sizeof (struct sockaddr_in);
-
- for (int i = 0; st[i].set; i++)
- {
- struct addrinfo *ai;
- ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
- if (ai == NULL)
- {
- result = -EAI_MEMORY;
- goto free_and_return;
- }
-
- ai->ai_flags = req->ai_flags;
- ai->ai_family = family;
- ai->ai_socktype = st[i].socktype;
- ai->ai_protocol = st[i].protocol;
- ai->ai_addrlen = socklen;
- ai->ai_addr = (void *) (ai + 1);
-
- /* We only add the canonical name once. */
- ai->ai_canonname = res.canon;
- res.canon = NULL;
-
-#ifdef _HAVE_SA_LEN
- ai->ai_addr->sa_len = socklen;
-#endif /* _HAVE_SA_LEN */
- ai->ai_addr->sa_family = family;
-
- /* In case of an allocation error the list must be NULL
- terminated. */
- ai->ai_next = NULL;
-
- if (family == AF_INET6)
- {
- struct sockaddr_in6 *sin6p =
- (struct sockaddr_in6 *) ai->ai_addr;
-
- sin6p->sin6_port = st[i].port;
- sin6p->sin6_flowinfo = 0;
- memcpy (&sin6p->sin6_addr,
- at2->addr, sizeof (struct in6_addr));
- sin6p->sin6_scope_id = at2->scopeid;
- }
- else
- {
- struct sockaddr_in *sinp =
- (struct sockaddr_in *) ai->ai_addr;
- sinp->sin_port = st[i].port;
- memcpy (&sinp->sin_addr,
- at2->addr, sizeof (struct in_addr));
- memset (sinp->sin_zero, '\0', sizeof (sinp->sin_zero));
- }
-
- pai = &(ai->ai_next);
- }
-
- ++*naddrs;
+ /* Set up the canonical name if we need it. */
+ if ((result = process_canonname (req, orig_name, &res)) != 0)
+ goto free_and_return;
- ignore:
- at2 = at2->next;
- }
- }
+ result = generate_addrinfo (req, &res, st, pai, naddrs);
- free_and_return:
+free_and_return:
if (malloc_name)
free ((char *) name);
free (addrmem);
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 12/13] gethosts: Return EAI_MEMORY on allocation failure
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (10 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 11/13] gaih_inet: Split result generation " Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
2023-09-15 23:47 ` [pushed 2.35 13/13] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) Siddhesh Poyarekar
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable; +Cc: DJ Delorie
All other cases of failures due to lack of memory return EAI_MEMORY, so
it seems wrong to return EAI_SYSTEM here. The only reason
convert_hostent_to_gaih_addrtuple could fail is on calloc failure.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Reviewed-by: DJ Delorie <dj@redhat.com>
(cherry picked from commit b587456c0e7b59dcfdbd2d44db000a3bc8244e57)
---
sysdeps/posix/getaddrinfo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index f5d4a5cfd9..0ece3b46b7 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -303,13 +303,13 @@ gethosts (nss_gethostbyname3_r fct, int family, const char *name,
else if (status == NSS_STATUS_SUCCESS)
{
if (!convert_hostent_to_gaih_addrtuple (req, family, &th, res))
- return -EAI_SYSTEM;
+ return -EAI_MEMORY;
if (localcanon != NULL && res->canon == NULL)
{
char *canonbuf = __strdup (localcanon);
if (canonbuf == NULL)
- return -EAI_SYSTEM;
+ return -EAI_MEMORY;
res->canon = canonbuf;
}
}
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [pushed 2.35 13/13] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806)
2023-09-15 23:47 [pushed 2.35 00/13] Backport gaih_inet refactoring and CVE fix Siddhesh Poyarekar
` (11 preceding siblings ...)
2023-09-15 23:47 ` [pushed 2.35 12/13] gethosts: Return EAI_MEMORY on allocation failure Siddhesh Poyarekar
@ 2023-09-15 23:47 ` Siddhesh Poyarekar
12 siblings, 0 replies; 14+ messages in thread
From: Siddhesh Poyarekar @ 2023-09-15 23:47 UTC (permalink / raw)
To: libc-stable
When an NSS plugin only implements the _gethostbyname2_r and
_getcanonname_r callbacks, getaddrinfo could use memory that was freed
during tmpbuf resizing, through h_name in a previous query response.
The backing store for res->at->name when doing a query with
gethostbyname3_r or gethostbyname2_r is tmpbuf, which is reallocated in
gethosts during the query. For AF_INET6 lookup with AI_ALL |
AI_V4MAPPED, gethosts gets called twice, once for a v6 lookup and second
for a v4 lookup. In this case, if the first call reallocates tmpbuf
enough number of times, resulting in a malloc, th->h_name (that
res->at->name refers to) ends up on a heap allocated storage in tmpbuf.
Now if the second call to gethosts also causes the plugin callback to
return NSS_STATUS_TRYAGAIN, tmpbuf will get freed, resulting in a UAF
reference in res->at->name. This then gets dereferenced in the
getcanonname_r plugin call, resulting in the use after free.
Fix this by copying h_name over and freeing it at the end. This
resolves BZ #30843, which is assigned CVE-2023-4806.
Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
(cherry picked from commit 973fe93a5675c42798b2161c6f29c01b0e243994)
---
nss/Makefile | 15 ++++-
nss/nss_test_gai_hv2_canonname.c | 56 +++++++++++++++++
nss/tst-nss-gai-hv2-canonname.c | 63 +++++++++++++++++++
nss/tst-nss-gai-hv2-canonname.h | 1 +
.../postclean.req | 0
.../tst-nss-gai-hv2-canonname.script | 2 +
sysdeps/posix/getaddrinfo.c | 25 +++++---
7 files changed, 152 insertions(+), 10 deletions(-)
create mode 100644 nss/nss_test_gai_hv2_canonname.c
create mode 100644 nss/tst-nss-gai-hv2-canonname.c
create mode 100644 nss/tst-nss-gai-hv2-canonname.h
create mode 100644 nss/tst-nss-gai-hv2-canonname.root/postclean.req
create mode 100644 nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
diff --git a/nss/Makefile b/nss/Makefile
index d8b06b44fb..ed1c05158e 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -80,6 +80,7 @@ tests-container := \
tst-nss-test3 \
tst-reload1 \
tst-reload2 \
+ tst-nss-gai-hv2-canonname \
# tests-container
# Tests which need libdl
@@ -143,7 +144,8 @@ libnss_compat-inhibit-o = $(filter-out .os,$(object-suffixes))
ifeq ($(build-static-nss),yes)
tests-static += tst-nss-static
endif
-extra-test-objs += nss_test1.os nss_test2.os nss_test_errno.os
+extra-test-objs += nss_test1.os nss_test2.os nss_test_errno.os \
+ nss_test_gai_hv2_canonname.os
include ../Rules
@@ -178,12 +180,16 @@ rtld-tests-LDFLAGS += -Wl,--dynamic-list=nss_test.ver
libof-nss_test1 = extramodules
libof-nss_test2 = extramodules
libof-nss_test_errno = extramodules
+libof-nss_test_gai_hv2_canonname = extramodules
$(objpfx)/libnss_test1.so: $(objpfx)nss_test1.os $(link-libc-deps)
$(build-module)
$(objpfx)/libnss_test2.so: $(objpfx)nss_test2.os $(link-libc-deps)
$(build-module)
$(objpfx)/libnss_test_errno.so: $(objpfx)nss_test_errno.os $(link-libc-deps)
$(build-module)
+$(objpfx)/libnss_test_gai_hv2_canonname.so: \
+ $(objpfx)nss_test_gai_hv2_canonname.os $(link-libc-deps)
+ $(build-module)
$(objpfx)nss_test2.os : nss_test1.c
# Use the nss_files suffix for these objects as well.
$(objpfx)/libnss_test1.so$(libnss_files.so-version): $(objpfx)/libnss_test1.so
@@ -193,10 +199,14 @@ $(objpfx)/libnss_test2.so$(libnss_files.so-version): $(objpfx)/libnss_test2.so
$(objpfx)/libnss_test_errno.so$(libnss_files.so-version): \
$(objpfx)/libnss_test_errno.so
$(make-link)
+$(objpfx)/libnss_test_gai_hv2_canonname.so$(libnss_files.so-version): \
+ $(objpfx)/libnss_test_gai_hv2_canonname.so
+ $(make-link)
$(patsubst %,$(objpfx)%.out,$(tests) $(tests-container)) : \
$(objpfx)/libnss_test1.so$(libnss_files.so-version) \
$(objpfx)/libnss_test2.so$(libnss_files.so-version) \
- $(objpfx)/libnss_test_errno.so$(libnss_files.so-version)
+ $(objpfx)/libnss_test_errno.so$(libnss_files.so-version) \
+ $(objpfx)/libnss_test_gai_hv2_canonname.so$(libnss_files.so-version)
ifeq (yes,$(have-thread-library))
$(objpfx)tst-cancel-getpwuid_r: $(shared-thread-library)
@@ -213,3 +223,4 @@ LDFLAGS-tst-nss-test3 = -Wl,--disable-new-dtags
LDFLAGS-tst-nss-test4 = -Wl,--disable-new-dtags
LDFLAGS-tst-nss-test5 = -Wl,--disable-new-dtags
LDFLAGS-tst-nss-test_errno = -Wl,--disable-new-dtags
+LDFLAGS-tst-nss-test_gai_hv2_canonname = -Wl,--disable-new-dtags
diff --git a/nss/nss_test_gai_hv2_canonname.c b/nss/nss_test_gai_hv2_canonname.c
new file mode 100644
index 0000000000..4439c83c9f
--- /dev/null
+++ b/nss/nss_test_gai_hv2_canonname.c
@@ -0,0 +1,56 @@
+/* NSS service provider that only provides gethostbyname2_r.
+ Copyright The GNU Toolchain Authors.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <nss.h>
+#include <stdlib.h>
+#include <string.h>
+#include "nss/tst-nss-gai-hv2-canonname.h"
+
+/* Catch misnamed and functions. */
+#pragma GCC diagnostic error "-Wmissing-prototypes"
+NSS_DECLARE_MODULE_FUNCTIONS (test_gai_hv2_canonname)
+
+extern enum nss_status _nss_files_gethostbyname2_r (const char *, int,
+ struct hostent *, char *,
+ size_t, int *, int *);
+
+enum nss_status
+_nss_test_gai_hv2_canonname_gethostbyname2_r (const char *name, int af,
+ struct hostent *result,
+ char *buffer, size_t buflen,
+ int *errnop, int *herrnop)
+{
+ return _nss_files_gethostbyname2_r (name, af, result, buffer, buflen, errnop,
+ herrnop);
+}
+
+enum nss_status
+_nss_test_gai_hv2_canonname_getcanonname_r (const char *name, char *buffer,
+ size_t buflen, char **result,
+ int *errnop, int *h_errnop)
+{
+ /* We expect QUERYNAME, which is a small enough string that it shouldn't fail
+ the test. */
+ if (memcmp (QUERYNAME, name, sizeof (QUERYNAME))
+ || buflen < sizeof (QUERYNAME))
+ abort ();
+
+ strncpy (buffer, name, buflen);
+ *result = buffer;
+ return NSS_STATUS_SUCCESS;
+}
diff --git a/nss/tst-nss-gai-hv2-canonname.c b/nss/tst-nss-gai-hv2-canonname.c
new file mode 100644
index 0000000000..d5f10c07d6
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.c
@@ -0,0 +1,63 @@
+/* Test NSS query path for plugins that only implement gethostbyname2
+ (#30843).
+ Copyright The GNU Toolchain Authors.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <nss.h>
+#include <netdb.h>
+#include <stdlib.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xstdio.h>
+#include "nss/tst-nss-gai-hv2-canonname.h"
+
+#define PREPARE do_prepare
+
+static void do_prepare (int a, char **av)
+{
+ FILE *hosts = xfopen ("/etc/hosts", "w");
+ for (unsigned i = 2; i < 255; i++)
+ {
+ fprintf (hosts, "ff01::ff02:ff03:%u:2\ttest.example.com\n", i);
+ fprintf (hosts, "192.168.0.%u\ttest.example.com\n", i);
+ }
+ xfclose (hosts);
+}
+
+static int
+do_test (void)
+{
+ __nss_configure_lookup ("hosts", "test_gai_hv2_canonname");
+
+ struct addrinfo hints = {};
+ struct addrinfo *result = NULL;
+
+ hints.ai_family = AF_INET6;
+ hints.ai_flags = AI_ALL | AI_V4MAPPED | AI_CANONNAME;
+
+ int ret = getaddrinfo (QUERYNAME, NULL, &hints, &result);
+
+ if (ret != 0)
+ FAIL_EXIT1 ("getaddrinfo failed: %s\n", gai_strerror (ret));
+
+ TEST_COMPARE_STRING (result->ai_canonname, QUERYNAME);
+
+ freeaddrinfo(result);
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/nss/tst-nss-gai-hv2-canonname.h b/nss/tst-nss-gai-hv2-canonname.h
new file mode 100644
index 0000000000..14f2a9cb08
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.h
@@ -0,0 +1 @@
+#define QUERYNAME "test.example.com"
diff --git a/nss/tst-nss-gai-hv2-canonname.root/postclean.req b/nss/tst-nss-gai-hv2-canonname.root/postclean.req
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
new file mode 100644
index 0000000000..31848b4a28
--- /dev/null
+++ b/nss/tst-nss-gai-hv2-canonname.root/tst-nss-gai-hv2-canonname.script
@@ -0,0 +1,2 @@
+cp $B/nss/libnss_test_gai_hv2_canonname.so $L/libnss_test_gai_hv2_canonname.so.2
+su
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 0ece3b46b7..ad7891a953 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -120,6 +120,7 @@ struct gaih_result
{
struct gaih_addrtuple *at;
char *canon;
+ char *h_name;
bool free_at;
bool got_ipv6;
};
@@ -165,6 +166,7 @@ gaih_result_reset (struct gaih_result *res)
if (res->free_at)
free (res->at);
free (res->canon);
+ free (res->h_name);
memset (res, 0, sizeof (*res));
}
@@ -203,9 +205,8 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
return 0;
}
-/* Convert struct hostent to a list of struct gaih_addrtuple objects. h_name
- is not copied, and the struct hostent object must not be deallocated
- prematurely. The new addresses are appended to the tuple array in RES. */
+/* Convert struct hostent to a list of struct gaih_addrtuple objects. The new
+ addresses are appended to the tuple array in RES. */
static bool
convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
struct hostent *h, struct gaih_result *res)
@@ -238,6 +239,15 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
res->at = array;
res->free_at = true;
+ /* Duplicate h_name because it may get reclaimed when the underlying storage
+ is freed. */
+ if (res->h_name == NULL)
+ {
+ res->h_name = __strdup (h->h_name);
+ if (res->h_name == NULL)
+ return false;
+ }
+
/* Update the next pointers on reallocation. */
for (size_t i = 0; i < old; i++)
array[i].next = array + i + 1;
@@ -262,7 +272,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
}
array[i].next = array + i + 1;
}
- array[0].name = h->h_name;
array[count - 1].next = NULL;
return true;
@@ -324,15 +333,15 @@ gethosts (nss_gethostbyname3_r fct, int family, const char *name,
memory allocation failure. The returned string is allocated on the
heap; the caller has to free it. */
static char *
-getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name)
+getcanonname (nss_action_list nip, const char *hname, const char *name)
{
nss_getcanonname_r *cfct = __nss_lookup_function (nip, "getcanonname_r");
char *s = (char *) name;
if (cfct != NULL)
{
char buf[256];
- if (DL_CALL_FCT (cfct, (at->name ?: name, buf, sizeof (buf),
- &s, &errno, &h_errno)) != NSS_STATUS_SUCCESS)
+ if (DL_CALL_FCT (cfct, (hname ?: name, buf, sizeof (buf), &s, &errno,
+ &h_errno)) != NSS_STATUS_SUCCESS)
/* If the canonical name cannot be determined, use the passed
string. */
s = (char *) name;
@@ -771,7 +780,7 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
if ((req->ai_flags & AI_CANONNAME) != 0
&& res->canon == NULL)
{
- char *canonbuf = getcanonname (nip, res->at, name);
+ char *canonbuf = getcanonname (nip, res->h_name, name);
if (canonbuf == NULL)
{
__resolv_context_put (res_ctx);
--
2.41.0
^ permalink raw reply [flat|nested] 14+ messages in thread