From patchwork Fri Mar 8 20:55:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1053711 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-100516-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="NKABOqCP"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44GKXT4w45z9s47 for ; Sat, 9 Mar 2019 07:55:53 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= PzP1rTZY89DR4ubbCYrXrF98fdgLbkncOj07xF3XoHHaPlOUg9lXea8XTqvZpqMk 7rfofvtjMYzPqrYckbT6zkCJQRlJfTaq6GU8Ptf/HwtSDK/KMTNS9nQV7/uOPYlu InuHHnxlf/YRL4UnnrXDp2jPw6c+UHjbzT5xvS3VUO0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=Ws/b+c mV2NXuXlWnUFKAcl+9xPU=; b=NKABOqCPcMRRK64luWWUtjPCy5ammE1m0hyolK bhmylAcbo9n/9451/57fNnqe+BAL/3QQdJdSXSNTEeBjXk50Bia5X4muu/KnvB/G WdUUfbSnS4nw5hkuZ+WE3ZqzrESCezlBW0vD+j24Bqmgab5hp9FVk7ohADJ++9kx dzn+U= Received: (qmail 120448 invoked by alias); 8 Mar 2019 20:55:47 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 119957 invoked by uid 89); 8 Mar 2019 20:55:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=85517, discarded, 8211 X-HELO: mx1.redhat.com Date: Fri, 08 Mar 2019 21:55:40 +0100 To: libc-alpha@sourceware.org Subject: [PATCH] nss_dns: Adjust gethostby* processing to struct alloc_buffer User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20190308205540.6D9F980DD6B5@oldenburg2.str.redhat.com> From: Florian Weimer In particular, this eliminates the struct host_data type in favor of explicit allocations from the buffer. _nss_dns_gethostbyaddr2_r used to poke into the struct host_data allocation made by getanswer_r, by carefully aligning pointers in the same way. This complexity is now gone. The old code used the bp pointer in key points to point to the textual version of the owner name of the current record. After the channges, the current_name variable assumes this task. This aspect of the code is rather tricky because to conserve buffer space, the allocation may actually be used for something else besides the decoded domain name (e.g. if the record is not a CNAME record and the name matches the previos name). 2019-03-08 Florian Weimer * resolv/nss_dns/dns-host.c (getanswer_r): Use struct alloc_buffer. (gethostbyname3_context): Adjust. (_nss_dns_gethostbyaddr2_r): Likewise. * resolv/mapv4v6hostent.h (align): Remove typedef. (map_v4v6_hostent): Use struct alloc_buffer. diff --git a/resolv/mapv4v6hostent.h b/resolv/mapv4v6hostent.h index c11038adf3..9addf37754 100644 --- a/resolv/mapv4v6hostent.h +++ b/resolv/mapv4v6hostent.h @@ -52,13 +52,8 @@ #include #include -typedef union { - int32_t al; - char ac; -} align; - static int -map_v4v6_hostent (struct hostent *hp, char **bpp, int *lenp) +map_v4v6_hostent (struct hostent *hp, struct alloc_buffer *abuffer) { char **ap; @@ -68,17 +63,15 @@ map_v4v6_hostent (struct hostent *hp, char **bpp, int *lenp) hp->h_length = IN6ADDRSZ; for (ap = hp->h_addr_list; *ap; ap++) { - int i = sizeof (align) - ((u_long) *bpp % sizeof (align)); - - if (*lenp < (i + IN6ADDRSZ)) + struct in6_addr *addr = alloc_buffer_alloc (abuffer, struct in6_addr); + if (addr == NULL) /* Out of memory. */ return 1; - *bpp += i; - *lenp -= i; - map_v4v6_address (*ap, *bpp); - *ap = *bpp; - *bpp += IN6ADDRSZ; - *lenp -= IN6ADDRSZ; + addr->s6_addr32[0] = htonl (0); + addr->s6_addr32[1] = htonl (0); + addr->s6_addr32[2] = htonl (0xffff); + memcpy (&addr->s6_addr32[3], *ap, sizeof (addr->s6_addr32[3])); + *ap = (char *) addr; } return 0; } diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c index f04e08016c..c5a2ca5e28 100644 --- a/resolv/nss_dns/dns-host.c +++ b/resolv/nss_dns/dns-host.c @@ -82,11 +82,12 @@ #include "nsswitch.h" #include +#include + #include #include /* Get implementations of some internal functions. */ -#include #include #define RESOLVSORT @@ -112,8 +113,9 @@ typedef union querybuf static enum nss_status getanswer_r (struct resolv_context *ctx, const querybuf *answer, int anslen, const char *qname, int qtype, - struct hostent *result, char *buffer, - size_t buflen, int *errnop, int *h_errnop, + struct hostent *result, + struct alloc_buffer *buffer, + int *errnop, int *h_errnop, int map, int32_t *ttlp, char **canonp); static enum nss_status gaih_getanswer (const querybuf *answer1, int anslen1, @@ -266,8 +268,9 @@ gethostbyname3_context (struct resolv_context *ctx, result->h_length = INADDRSZ; } + struct alloc_buffer abuffer = alloc_buffer_create (buffer, buflen); status = getanswer_r - (ctx, host_buffer.buf, n, name, type, result, buffer, buflen, + (ctx, host_buffer.buf, n, name, type, result, &abuffer, errnop, h_errnop, map, ttlp, canonp); if (host_buffer.buf != orig_host_buffer) free (host_buffer.buf); @@ -435,13 +438,6 @@ _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af, static const u_char tunnelled[] = { 0,0, 0,0, 0,0, 0,0, 0,0, 0,0 }; static const u_char v6local[] = { 0,0, 0,1 }; const u_char *uaddr = (const u_char *)addr; - struct host_data - { - char *aliases[MAX_NR_ALIASES]; - unsigned char host_addr[16]; /* IPv4 or IPv6 */ - char *h_addr_ptrs[MAX_NR_ADDRS + 1]; - char linebuffer[0]; - } *host_data = (struct host_data *) buffer; union { querybuf *buf; @@ -453,19 +449,6 @@ _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af, int n, status; int olderr = errno; - uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct host_data); - buffer += pad; - buflen = buflen > pad ? buflen - pad : 0; - - if (__glibc_unlikely (buflen < sizeof (struct host_data))) - { - *errnop = ERANGE; - *h_errnop = NETDB_INTERNAL; - return NSS_STATUS_TRYAGAIN; - } - - host_data = (struct host_data *) buffer; - struct resolv_context *ctx = __resolv_context_get (); if (ctx == NULL) { @@ -545,8 +528,9 @@ _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af, return errno == ECONNREFUSED ? NSS_STATUS_UNAVAIL : NSS_STATUS_NOTFOUND; } + struct alloc_buffer abuffer = alloc_buffer_create (buffer, buflen); status = getanswer_r - (ctx, host_buffer.buf, n, qbuf, T_PTR, result, buffer, buflen, + (ctx, host_buffer.buf, n, qbuf, T_PTR, result, &abuffer, errnop, h_errnop, 0 /* XXX */, ttlp, NULL); if (host_buffer.buf != orig_host_buffer) free (host_buffer.buf); @@ -558,9 +542,21 @@ _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af, result->h_addrtype = af; result->h_length = len; - memcpy (host_data->host_addr, addr, len); - host_data->h_addr_ptrs[0] = (char *) host_data->host_addr; - host_data->h_addr_ptrs[1] = NULL; + result->h_addr_list = alloc_buffer_alloc_array (&abuffer, char *, 2); + if (result->h_addr_list != NULL) + { + char *host_addr = __alloc_buffer_alloc (&abuffer, len, + __alignof (struct in_addr)); + memcpy (host_addr, addr, len); + result->h_addr_list[0] = host_addr; + result->h_addr_list[1] = NULL; + } + if (alloc_buffer_has_failed (&abuffer)) + { + *errnop = ERANGE; + *h_errnop = NETDB_INTERNAL; + return NSS_STATUS_TRYAGAIN; + } *h_errnop = NETDB_SUCCESS; __resolv_context_put (ctx); return NSS_STATUS_SUCCESS; @@ -626,41 +622,19 @@ addrsort (struct resolv_context *ctx, char **ap, int num) static enum nss_status getanswer_r (struct resolv_context *ctx, const querybuf *answer, int anslen, const char *qname, int qtype, - struct hostent *result, char *buffer, size_t buflen, + struct hostent *result, struct alloc_buffer *buffer, int *errnop, int *h_errnop, int map, int32_t *ttlp, char **canonp) { - struct host_data - { - char *aliases[MAX_NR_ALIASES]; - unsigned char host_addr[16]; /* IPv4 or IPv6 */ - char *h_addr_ptrs[0]; - } *host_data; - int linebuflen; const HEADER *hp; const u_char *end_of_message, *cp; int n, ancount, qdcount; int haveanswer, had_error; - char *bp, **ap, **hap; + char **ap, **hap; char tbuf[MAXDNAME]; const char *tname; int (*name_ok) (const char *); u_char packtmp[NS_MAXCDNAME]; int have_to_map = 0; - uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct host_data); - buffer += pad; - buflen = buflen > pad ? buflen - pad : 0; - if (__glibc_unlikely (buflen < sizeof (struct host_data))) - { - /* The buffer is too small. */ - too_small: - *errnop = ERANGE; - *h_errnop = NETDB_INTERNAL; - return NSS_STATUS_TRYAGAIN; - } - host_data = (struct host_data *) buffer; - linebuflen = buflen - sizeof (struct host_data); - if (buflen - sizeof (struct host_data) != linebuflen) - linebuflen = INT_MAX; tname = qname; result->h_name = NULL; @@ -691,10 +665,18 @@ getanswer_r (struct resolv_context *ctx, *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; } - if (sizeof (struct host_data) + (ancount + 1) * sizeof (char *) >= buflen) - goto too_small; - bp = (char *) &host_data->h_addr_ptrs[ancount + 1]; - linebuflen -= (ancount + 1) * sizeof (char *); + + result->h_aliases = alloc_buffer_alloc_array (buffer, char *, + MAX_NR_ALIASES); + result->h_addr_list = alloc_buffer_alloc_array (buffer, char *, + ancount + 1); + if (result->h_addr_list == NULL) + { + too_small: + *errnop = ERANGE; + *h_errnop = NETDB_INTERNAL; + return NSS_STATUS_TRYAGAIN; + } n = __ns_name_unpack (answer->buf, end_of_message, cp, packtmp, sizeof packtmp); @@ -704,13 +686,21 @@ getanswer_r (struct resolv_context *ctx, *h_errnop = NO_RECOVERY; return NSS_STATUS_UNAVAIL; } - if (__ns_name_ntop (packtmp, bp, linebuflen) < 0) + + if (__ns_name_ntop (packtmp, alloc_buffer_next (buffer, char), + alloc_buffer_size (buffer)) < 0) goto too_small; - if (bp[0] == '.') - bp[0] = '\0'; + /* Store the currently processed name in the unallocated tail of the + buffer. In subsequent iterations of the loop, the decoded name + may be discarded if it is not needed after all, and current_name + becomes invalid at this point. */ + char *current_name = alloc_buffer_next (buffer, char); - if (__glibc_unlikely (name_ok (bp) == 0)) + if (current_name[0] == '.') + current_name[0] = '\0'; + + if (__glibc_unlikely (name_ok (current_name) == 0)) { errno = EBADMSG; *errnop = EBADMSG; @@ -725,28 +715,23 @@ getanswer_r (struct resolv_context *ctx, * same as the one we sent; this just gets the expanded name * (i.e., with the succeeding search-domain tacked on). */ - n = strlen (bp) + 1; /* for the \0 */ + n = strlen (current_name) + 1; /* For the '\0'. */ if (n >= MAXHOSTNAMELEN) { *h_errnop = NO_RECOVERY; *errnop = ENOENT; return NSS_STATUS_TRYAGAIN; } - result->h_name = bp; - bp += n; - linebuflen -= n; - if (linebuflen < 0) - goto too_small; + result->h_name = current_name; + alloc_buffer_alloc_bytes (buffer, n); /* The qname can be abbreviated, but h_name is now absolute. */ qname = result->h_name; } - ap = host_data->aliases; + ap = result->h_aliases; *ap = NULL; - result->h_aliases = host_data->aliases; - hap = host_data->h_addr_ptrs; + hap = result->h_addr_list; *hap = NULL; - result->h_addr_list = host_data->h_addr_ptrs; haveanswer = 0; had_error = 0; @@ -762,10 +747,12 @@ getanswer_r (struct resolv_context *ctx, continue; } - if (__ns_name_ntop (packtmp, bp, linebuflen) < 0) + if (__ns_name_ntop (packtmp, alloc_buffer_next (buffer, char), + alloc_buffer_size (buffer)) < 0) goto too_small; + current_name = alloc_buffer_next (buffer, char); - if (__glibc_unlikely ((*name_ok) (bp) == 0)) + if (__glibc_unlikely ((*name_ok) (current_name) == 0)) { ++had_error; continue; @@ -807,7 +794,7 @@ getanswer_r (struct resolv_context *ctx, if (ttlp != NULL && ttl < *ttlp) *ttlp = ttl; - if (ap >= &host_data->aliases[MAX_NR_ALIASES - 1]) + if (ap >= &result->h_aliases[MAX_NR_ALIASES - 1]) continue; n = dn_expand (answer->buf, end_of_message, cp, tbuf, sizeof tbuf); if (__glibc_unlikely (n < 0 || (*name_ok) (tbuf) == 0)) @@ -817,27 +804,24 @@ getanswer_r (struct resolv_context *ctx, } cp += n; /* Store alias. */ - *ap++ = bp; - n = strlen (bp) + 1; /* For the \0. */ + *ap++ = current_name; + n = strlen (current_name) + 1; /* For the \0. */ if (__glibc_unlikely (n >= MAXHOSTNAMELEN)) { ++had_error; continue; } - bp += n; - linebuflen -= n; + alloc_buffer_alloc_bytes (buffer, n); /* Get canonical name. */ - n = strlen (tbuf) + 1; /* For the \0. */ - if (__glibc_unlikely (n > linebuflen)) + result->h_name = alloc_buffer_copy_string (buffer, tbuf); + current_name = NULL; + if (result->h_name == NULL) goto too_small; - if (__glibc_unlikely (n >= MAXHOSTNAMELEN)) + if (strlen (tbuf) >= MAXHOSTNAMELEN) { ++had_error; continue; } - result->h_name = bp; - bp = __mempcpy (bp, tbuf, n); /* Cannot overflow. */ - linebuflen -= n; continue; } @@ -855,17 +839,15 @@ getanswer_r (struct resolv_context *ctx, } cp += n; /* Get canonical name. */ - n = strlen (tbuf) + 1; /* For the \0. */ - if (__glibc_unlikely (n > linebuflen)) + tname = alloc_buffer_copy_string (buffer, tbuf); + if (tname == NULL) goto too_small; - if (__glibc_unlikely (n >= MAXHOSTNAMELEN)) + if (strlen (tbuf) >= MAXHOSTNAMELEN) { ++had_error; continue; } - tname = bp; - bp = __mempcpy (bp, tbuf, n); /* Cannot overflow. */ - linebuflen -= n; + current_name = NULL; continue; } @@ -880,7 +862,8 @@ getanswer_r (struct resolv_context *ctx, switch (type) { case T_PTR: - if (__glibc_unlikely (strcasecmp (tname, bp) != 0)) + if (__glibc_unlikely (strcasecmp (tname, current_name) + != 0)) { cp += n; continue; /* XXX - had_error++ ? */ @@ -893,9 +876,14 @@ getanswer_r (struct resolv_context *ctx, ++had_error; break; } - if (__ns_name_ntop (packtmp, bp, linebuflen) < 0) + + if (__ns_name_ntop (packtmp, + alloc_buffer_next (buffer, char), + alloc_buffer_size (buffer)) < 0) goto too_small; - if (__glibc_unlikely (res_hnok (bp) == 0)) + current_name = alloc_buffer_next (buffer, char); + + if (__glibc_unlikely (res_hnok (current_name) == 0)) { ++had_error; break; @@ -904,12 +892,14 @@ getanswer_r (struct resolv_context *ctx, *ttlp = ttl; /* bind would put multiple PTR records as aliases, but we don't do that. */ - result->h_name = bp; + result->h_name = alloc_buffer_next (buffer, char); + alloc_buffer_alloc_bytes (buffer, strlen (result->h_name) + 1); *h_errnop = NETDB_SUCCESS; return NSS_STATUS_SUCCESS; case T_A: case T_AAAA: - if (__glibc_unlikely (strcasecmp (result->h_name, bp) != 0)) + if (__glibc_unlikely (strcasecmp (result->h_name, current_name) + != 0)) { cp += n; continue; /* XXX - had_error++ ? */ @@ -930,29 +920,26 @@ getanswer_r (struct resolv_context *ctx, } if (!haveanswer) { - int nn; - /* We compose a single hostent out of the entire chain of entries, so the TTL of the hostent is essentially the lowest TTL in the chain. */ if (ttlp != NULL && ttl < *ttlp) *ttlp = ttl; if (canonp != NULL) - *canonp = bp; - result->h_name = bp; - nn = strlen (bp) + 1; /* for the \0 */ - bp += nn; - linebuflen -= nn; + *canonp = alloc_buffer_next (buffer, char); + result->h_name = alloc_buffer_next (buffer, char); + alloc_buffer_alloc_bytes (buffer, strlen (result->h_name) + 1); } - linebuflen -= sizeof (align) - ((u_long) bp % sizeof (align)); - bp += sizeof (align) - ((u_long) bp % sizeof (align)); - - if (__glibc_unlikely (n > linebuflen)) - goto too_small; - bp = __mempcpy (*hap++ = bp, cp, n); - cp += n; - linebuflen -= n; + { + void *addr = __alloc_buffer_alloc (buffer, n, + __alignof (struct in_addr)); + if (addr == NULL) + goto too_small; + memcpy (addr, cp, n); + *hap++ = addr; + cp += n; + } break; default: abort (); @@ -972,22 +959,19 @@ getanswer_r (struct resolv_context *ctx, */ if (haveanswer > 1 && qtype == T_A && __resolv_context_sort_count (ctx) > 0) - addrsort (ctx, host_data->h_addr_ptrs, haveanswer); + addrsort (ctx, result->h_addr_list, haveanswer); if (result->h_name == NULL) { - n = strlen (qname) + 1; /* For the \0. */ - if (n > linebuflen) + result->h_name = alloc_buffer_copy_string (buffer, qname); + if (result->h_name == NULL) goto too_small; - if (n >= MAXHOSTNAMELEN) + if (strlen (qname) + 1 >= MAXHOSTNAMELEN) goto no_recovery; - result->h_name = bp; - bp = __mempcpy (bp, qname, n); /* Cannot overflow. */ - linebuflen -= n; } if (have_to_map) - if (map_v4v6_hostent (result, &bp, &linebuflen)) + if (map_v4v6_hostent (result, buffer)) goto too_small; *h_errnop = NETDB_SUCCESS; return NSS_STATUS_SUCCESS; @@ -998,7 +982,7 @@ getanswer_r (struct resolv_context *ctx, /* Special case here: if the resolver sent a result but it only contains a CNAME while we are looking for a T_A or T_AAAA record, we fail with NOTFOUND instead of TRYAGAIN. */ - return ((qtype == T_A || qtype == T_AAAA) && ap != host_data->aliases + return ((qtype == T_A || qtype == T_AAAA) && ap != result->h_aliases ? NSS_STATUS_NOTFOUND : NSS_STATUS_TRYAGAIN); }