From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from insect.birch.relay.mailchannels.net (insect.birch.relay.mailchannels.net [23.83.209.93]) by sourceware.org (Postfix) with ESMTPS id 4BCA83858D32 for ; Mon, 22 Aug 2022 16:18:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4BCA83858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id E1B9781847; Mon, 22 Aug 2022 16:18:17 +0000 (UTC) Received: from pdx1-sub0-mail-a306.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 731CE81C26; Mon, 22 Aug 2022 16:18:17 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1661185097; a=rsa-sha256; cv=none; b=gMH5yTsyYfX/L60HTVAUtZ8y0EFe3G8R5MKHee8SQy6v18ZEiBhUWITj3xTwvXzTEYFsSY 0JP0PpVMBgeoBLdCw+CXnv25M2ZosSKN3Mqy4odhFDyt+39INtcJQIRxcsId29kDjAbOvh lkjmmoNYgSL6AGKivKZYnmVIg3GPoFATxfnBycCVjC5uOZuLmzGE5aS2myJ23XMc5MX3wl e0J/mtWZuVIyZmNI4lusu8aUXzfUCMfsUGrVYLrd9ED1eGLyxwYeGrSWnUffcMuL4Amr3N NZHHcTwqQo47yFvGi5EFHR/WtyyLKLHhVZDuI88sX5I9iY7Eb/cx+f1AHaKYDg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1661185097; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=MJhke5zI+a0OAheqqepnvA/bxo+JMMglqN0Zwic2bsc=; b=UdWlfbm0+1tJvkIpahapJCayurFf7d9NJjUcY4tkaSWxXpZb93Z8UPiacWb/EfHUbukwpO mYDSCWm1Y1IQFQ15A/KqeX1PgWOb6zJWiXoyEC2UQRrlifWd7Y6ILcRUdea7hFtmAvmF0O HXdNsrzQZ0Lw7eFuXvgoSrcUB8RXhAsuZmyZAhtkREmC0tYM7S8jLSsRWcwF3+c01/zHFJ 0o6GT37IqnJw8lqiDMY3fTDR5EQ6MjPYW5X+ZGhOTijQ94rPXHuW8bPzbDaZzhZkzzMwuj t1Pk76xl+GvOvPab+OqrbFu9Q5CO1G8dEpGnluYYTNYqGtdUZtYQOM9L2G/MsQ== ARC-Authentication-Results: i=1; rspamd-769cfffc99-gz2kr; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Wipe-Irritate: 59e52694485579ba_1661185097732_2516232966 X-MC-Loop-Signature: 1661185097732:3997448587 X-MC-Ingress-Time: 1661185097732 Received: from pdx1-sub0-mail-a306.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.115.45.14 (trex/6.7.1); Mon, 22 Aug 2022 16:18:17 +0000 Received: from [192.168.0.182] (bras-vprn-toroon4834w-lp130-16-184-147-84-238.dsl.bell.ca [184.147.84.238]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a306.dreamhost.com (Postfix) with ESMTPSA id 4MBHbD4cC6zFP; Mon, 22 Aug 2022 09:18:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1661185097; bh=MJhke5zI+a0OAheqqepnvA/bxo+JMMglqN0Zwic2bsc=; h=Date:Subject:To:From:Content-Type:Content-Transfer-Encoding; b=AFDTb9uH6qcjVrD8DABk5DjsZ4tG5wfrpQhVf3f4lcdGKkrwOrGKzn+f8cE5xK3xn qFsZ51WAelByeOlgKu9go49hPt1Nu7mqzgaUu1WVt6Nx2DrpRCBfar6F8lF3JCHchw c6n7cRmzalmbfsQytOOUNyA9lnTetiOEvEuiE1XRsaqfWpva9Dy0GmldhAv3qynx6x 15foyGJuz20y6JklyKRU4fd1g1e4GkubRqd9fMkNGlxAvkiTp9tj/6O13TL+yeFl+p FFNWRxSuiiUT1UozPDN3g6EPPXsGFNsyD+doP9Omt0OD9rog+MTisadO8ItAoXy7V+ G6kVcdbUT9gFA== Message-ID: <9b7a128b-200f-3db6-2933-5dfa29d836ef@gotplt.org> Date: Mon, 22 Aug 2022 12:18:15 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH 07/13] nss_dns: Split getanswer_ptr from getanswer_r Content-Language: en-US To: Florian Weimer , libc-alpha@sourceware.org References: From: Siddhesh Poyarekar In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3038.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Aug 2022 16:18:26 -0000 On 2022-08-10 05:30, Florian Weimer via Libc-alpha wrote: > And expand the use of name_ok and qtype in getanswer_ptr (the > former also in getanswer_r). > > After further cleanups, not much code will be shared between the > two functions. > --- > resolv/nss_dns/dns-host.c | 320 +++++++++++++++++++++++++++++++------- > 1 file changed, 268 insertions(+), 52 deletions(-) > Reviewed patch with git diff --anchored="getanswer_r (const querybuf *answer, int anslen, const char *qname," and it looks functionally same as before. LGTM. Reviewed-by: Siddhesh Poyarekar > diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c > index 544cffbecd..d384e1f82d 100644 > --- a/resolv/nss_dns/dns-host.c > +++ b/resolv/nss_dns/dns-host.c > @@ -116,6 +116,11 @@ static enum nss_status getanswer_r (struct resolv_context *ctx, > struct hostent *result, char *buffer, > size_t buflen, int *errnop, int *h_errnop, > int map, int32_t *ttlp, char **canonp); > +static enum nss_status getanswer_ptr (const querybuf *answer, int anslen, > + const char *qname, > + struct hostent *result, char *buffer, > + size_t buflen, int *errnop, > + int *h_errnop, int32_t *ttlp); > > static enum nss_status gaih_getanswer (const querybuf *answer1, int anslen1, > const querybuf *answer2, int anslen2, > @@ -561,9 +566,8 @@ _nss_dns_gethostbyaddr2_r (const void *addr, socklen_t len, int af, > return errno == ECONNREFUSED ? NSS_STATUS_UNAVAIL : NSS_STATUS_NOTFOUND; > } > > - status = getanswer_r > - (ctx, host_buffer.buf, n, qbuf, T_PTR, result, buffer, buflen, > - errnop, h_errnop, 0 /* XXX */, ttlp, NULL); > + status = getanswer_ptr (host_buffer.buf, n, qbuf, result, > + buffer, buflen, errnop, h_errnop, ttlp); > if (host_buffer.buf != orig_host_buffer) > free (host_buffer.buf); > if (status != NSS_STATUS_SUCCESS) > @@ -659,8 +663,6 @@ getanswer_r (struct resolv_context *ctx, > int haveanswer, had_error; > char *bp, **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); > @@ -679,22 +681,8 @@ getanswer_r (struct resolv_context *ctx, > if (buflen - sizeof (struct host_data) != linebuflen) > linebuflen = INT_MAX; > > - tname = qname; > result->h_name = NULL; > end_of_message = answer->buf + anslen; > - switch (qtype) > - { > - case T_A: > - case T_AAAA: > - name_ok = __libc_res_hnok; > - break; > - case T_PTR: > - name_ok = __libc_res_dnok; > - break; > - default: > - *errnop = ENOENT; > - return NSS_STATUS_UNAVAIL; /* XXX should be abort(); */ > - } > > /* > * find first satisfactory answer > @@ -729,7 +717,7 @@ getanswer_r (struct resolv_context *ctx, > *h_errnop = NO_RECOVERY; > return NSS_STATUS_UNAVAIL; > } > - if (__glibc_unlikely (name_ok (bp) == 0)) > + if (__glibc_unlikely (__libc_res_hnok (bp) == 0)) > { > errno = EBADMSG; > *errnop = EBADMSG; > @@ -783,7 +771,7 @@ getanswer_r (struct resolv_context *ctx, > n = -1; > } > > - if (__glibc_unlikely (n < 0 || (*name_ok) (bp) == 0)) > + if (__glibc_unlikely (n < 0 || __libc_res_hnok (bp) == 0)) > { > ++had_error; > continue; > @@ -816,7 +804,7 @@ getanswer_r (struct resolv_context *ctx, > continue; /* XXX - had_error++ ? */ > } > > - if ((qtype == T_A || qtype == T_AAAA) && type == T_CNAME) > + if (type == T_CNAME) > { > /* A CNAME could also have a TTL entry. */ > if (ttlp != NULL && ttl < *ttlp) > @@ -826,7 +814,7 @@ getanswer_r (struct resolv_context *ctx, > continue; > n = __libc_dn_expand (answer->buf, end_of_message, cp, > tbuf, sizeof tbuf); > - if (__glibc_unlikely (n < 0 || (*name_ok) (tbuf) == 0)) > + if (__glibc_unlikely (n < 0 || __libc_res_hnok (tbuf) == 0)) > { > ++had_error; > continue; > @@ -857,7 +845,260 @@ getanswer_r (struct resolv_context *ctx, > continue; > } > > - if (qtype == T_PTR && type == T_CNAME) > + if (type == T_A && qtype == T_AAAA && map) > + have_to_map = 1; > + else if (__glibc_unlikely (type != qtype)) > + { > + cp += n; > + continue; /* XXX - had_error++ ? */ > + } > + > + switch (type) > + { > + case T_A: > + case T_AAAA: > + if (__glibc_unlikely (__strcasecmp (result->h_name, bp) != 0)) > + { > + cp += n; > + continue; /* XXX - had_error++ ? */ > + } > + > + /* Stop parsing at a record whose length is incorrect. */ > + if (n != rrtype_to_rdata_length (type)) > + { > + ++had_error; > + break; > + } > + > + /* Skip records of the wrong type. */ > + if (n != result->h_length) > + { > + cp += n; > + continue; > + } > + 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; > + } > + > + /* Provide sufficient alignment for both address > + families. */ > + enum { align = 4 }; > + _Static_assert ((align % __alignof__ (struct in_addr)) == 0, > + "struct in_addr alignment"); > + _Static_assert ((align % __alignof__ (struct in6_addr)) == 0, > + "struct in6_addr alignment"); > + { > + char *new_bp = PTR_ALIGN_UP (bp, align); > + linebuflen -= new_bp - bp; > + bp = new_bp; > + } > + > + if (__glibc_unlikely (n > linebuflen)) > + goto too_small; > + bp = __mempcpy (*hap++ = bp, cp, n); > + cp += n; > + linebuflen -= n; > + break; > + default: > + abort (); > + } > + if (had_error == 0) > + ++haveanswer; > + } > + > + if (haveanswer > 0) > + { > + *ap = NULL; > + *hap = NULL; > + /* > + * Note: we sort even if host can take only one address > + * in its return structures - should give it the "best" > + * address in that case, not some random one > + */ > + if (haveanswer > 1 && qtype == T_A > + && __resolv_context_sort_count (ctx) > 0) > + addrsort (ctx, host_data->h_addr_ptrs, haveanswer); > + > + if (result->h_name == NULL) > + { > + n = strlen (qname) + 1; /* For the \0. */ > + if (n > linebuflen) > + goto too_small; > + if (n >= 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)) > + goto too_small; > + *h_errnop = NETDB_SUCCESS; > + return NSS_STATUS_SUCCESS; > + } > + no_recovery: > + *h_errnop = NO_RECOVERY; > + *errnop = ENOENT; > + /* 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 > + ? NSS_STATUS_NOTFOUND : NSS_STATUS_TRYAGAIN); > +} > + > +static enum nss_status > +getanswer_ptr (const querybuf *answer, int anslen, const char *qname, > + struct hostent *result, char *buffer, size_t buflen, > + int *errnop, int *h_errnop, int32_t *ttlp) > +{ > + 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 tbuf[MAXDNAME]; > + const char *tname; > + u_char packtmp[NS_MAXCDNAME]; > + 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; > + end_of_message = answer->buf + anslen; > + > + /* > + * find first satisfactory answer > + */ > + hp = &answer->hdr; > + ancount = ntohs (hp->ancount); > + qdcount = ntohs (hp->qdcount); > + cp = answer->buf + HFIXEDSZ; > + if (__glibc_unlikely (qdcount != 1)) > + { > + *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 *); > + > + n = __ns_name_unpack (answer->buf, end_of_message, cp, > + packtmp, sizeof packtmp); > + if (n != -1 && __ns_name_ntop (packtmp, bp, linebuflen) == -1) > + { > + if (__glibc_unlikely (errno == EMSGSIZE)) > + goto too_small; > + > + n = -1; > + } > + > + if (__glibc_unlikely (n < 0)) > + { > + *errnop = errno; > + *h_errnop = NO_RECOVERY; > + return NSS_STATUS_UNAVAIL; > + } > + if (__glibc_unlikely (__libc_res_dnok (bp) == 0)) > + { > + errno = EBADMSG; > + *errnop = EBADMSG; > + *h_errnop = NO_RECOVERY; > + return NSS_STATUS_UNAVAIL; > + } > + cp += n + QFIXEDSZ; > + > + ap = host_data->aliases; > + *ap = NULL; > + result->h_aliases = host_data->aliases; > + hap = host_data->h_addr_ptrs; > + *hap = NULL; > + result->h_addr_list = host_data->h_addr_ptrs; > + haveanswer = 0; > + had_error = 0; > + > + while (ancount-- > 0 && cp < end_of_message && had_error == 0) > + { > + int type, class; > + > + n = __ns_name_unpack (answer->buf, end_of_message, cp, > + packtmp, sizeof packtmp); > + if (n != -1 && __ns_name_ntop (packtmp, bp, linebuflen) == -1) > + { > + if (__glibc_unlikely (errno == EMSGSIZE)) > + goto too_small; > + > + n = -1; > + } > + > + if (__glibc_unlikely (n < 0 || __libc_res_dnok (bp) == 0)) > + { > + ++had_error; > + continue; > + } > + cp += n; /* name */ > + > + if (__glibc_unlikely (cp + 10 > end_of_message)) > + { > + ++had_error; > + continue; > + } > + > + NS_GET16 (type, cp); > + NS_GET16 (class, cp); > + int32_t ttl; > + NS_GET32 (ttl, cp); > + NS_GET16 (n, cp); /* RDATA length. */ > + > + if (end_of_message - cp < n) > + { > + /* RDATA extends beyond the end of the packet. */ > + ++had_error; > + continue; > + } > + > + if (__glibc_unlikely (class != C_IN)) > + { > + /* XXX - debug? syslog? */ > + cp += n; > + continue; /* XXX - had_error++ ? */ > + } > + > + if (type == T_CNAME) > { > /* A CNAME could also have a TTL entry. */ > if (ttlp != NULL && ttl < *ttlp) > @@ -886,14 +1127,6 @@ getanswer_r (struct resolv_context *ctx, > continue; > } > > - if (type == T_A && qtype == T_AAAA && map) > - have_to_map = 1; > - else if (__glibc_unlikely (type != qtype)) > - { > - cp += n; > - continue; /* XXX - had_error++ ? */ > - } > - > switch (type) > { > case T_PTR: > @@ -955,8 +1188,6 @@ getanswer_r (struct resolv_context *ctx, > 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; > @@ -983,7 +1214,8 @@ getanswer_r (struct resolv_context *ctx, > linebuflen -= n; > break; > default: > - abort (); > + cp += n; > + continue; /* XXX - had_error++ ? */ > } > if (had_error == 0) > ++haveanswer; > @@ -993,14 +1225,6 @@ getanswer_r (struct resolv_context *ctx, > { > *ap = NULL; > *hap = NULL; > - /* > - * Note: we sort even if host can take only one address > - * in its return structures - should give it the "best" > - * address in that case, not some random one > - */ > - if (haveanswer > 1 && qtype == T_A > - && __resolv_context_sort_count (ctx) > 0) > - addrsort (ctx, host_data->h_addr_ptrs, haveanswer); > > if (result->h_name == NULL) > { > @@ -1014,23 +1238,15 @@ getanswer_r (struct resolv_context *ctx, > linebuflen -= n; > } > > - if (have_to_map) > - if (map_v4v6_hostent (result, &bp, &linebuflen)) > - goto too_small; > *h_errnop = NETDB_SUCCESS; > return NSS_STATUS_SUCCESS; > } > no_recovery: > *h_errnop = NO_RECOVERY; > *errnop = ENOENT; > - /* 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 > - ? NSS_STATUS_NOTFOUND : NSS_STATUS_TRYAGAIN); > + return NSS_STATUS_TRYAGAIN; > } > > - > static enum nss_status > gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname, > struct gaih_addrtuple ***patp,