From 6b7b2abac75f969a86c537d64adf003378e24113 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Mon, 25 Jun 2018 16:05:46 +0200 Subject: nscd: Switch to struct scratch_buffer in adhstaiX [BZ #18023] The pre-allocation of the three scratch buffers increased the initial stack size somewhat, but if retries are needed, the previous version used more stack space if extend_alloca could not merge allocations. Lack of alloca accounting also means could be problematic with extremely large NSS responses, too. [BZ #18023] * nscd/aicache.c (addhstaiX): Use struct scratch_buffer instead of extend_alloca. --- ChangeLog | 6 +++++ nscd/aicache.c | 79 +++++++++++++++++++++++++++++++++------------------------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5b1c0315e0..1cafbb1882 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2018-06-25 Florian Weimer + + [BZ #18023] + * nscd/aicache.c (addhstaiX): Use struct scratch_buffer instead + of extend_alloca. + 2018-06-25 Florian Weimer [BZ #18023] diff --git a/nscd/aicache.c b/nscd/aicache.c index 2095edf911..439e0ea126 100644 --- a/nscd/aicache.c +++ b/nscd/aicache.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "dbg_log.h" #include "nscd.h" @@ -108,10 +109,13 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, if (ctx == NULL) no_more = 1; - size_t tmpbuf6len = 1024; - char *tmpbuf6 = alloca (tmpbuf6len); - size_t tmpbuf4len = 0; - char *tmpbuf4 = NULL; + struct scratch_buffer tmpbuf6; + scratch_buffer_init (&tmpbuf6); + struct scratch_buffer tmpbuf4; + scratch_buffer_init (&tmpbuf4); + struct scratch_buffer canonbuf; + scratch_buffer_init (&canonbuf); + int32_t ttl = INT32_MAX; ssize_t total = 0; char *key_copy = NULL; @@ -124,6 +128,7 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, int status[2] = { NSS_STATUS_UNAVAIL, NSS_STATUS_UNAVAIL }; int naddrs = 0; size_t addrslen = 0; + char *canon = NULL; size_t canonlen; @@ -138,12 +143,17 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, at = &atmem; rc6 = 0; herrno = 0; - status[1] = DL_CALL_FCT (fct4, (key, &at, tmpbuf6, tmpbuf6len, + status[1] = DL_CALL_FCT (fct4, (key, &at, + tmpbuf6.data, tmpbuf6.length, &rc6, &herrno, &ttl)); if (rc6 != ERANGE || (herrno != NETDB_INTERNAL && herrno != TRY_AGAIN)) break; - tmpbuf6 = extend_alloca (tmpbuf6, tmpbuf6len, 2 * tmpbuf6len); + if (!scratch_buffer_grow (&tmpbuf6)) + { + rc6 = ENOMEM; + break; + } } if (rc6 != 0 && herrno == NETDB_INTERNAL) @@ -221,41 +231,38 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, while (1) { rc6 = 0; - status[0] = DL_CALL_FCT (fct, (key, AF_INET6, &th[0], tmpbuf6, - tmpbuf6len, &rc6, &herrno, &ttl, + status[0] = DL_CALL_FCT (fct, (key, AF_INET6, &th[0], + tmpbuf6.data, tmpbuf6.length, + &rc6, &herrno, &ttl, &canon)); if (rc6 != ERANGE || herrno != NETDB_INTERNAL) break; - tmpbuf6 = extend_alloca (tmpbuf6, tmpbuf6len, 2 * tmpbuf6len); + if (!scratch_buffer_grow (&tmpbuf6)) + { + rc6 = ENOMEM; + break; + } } if (rc6 != 0 && herrno == NETDB_INTERNAL) goto out; - /* If the IPv6 lookup has been successful do not use the - buffer used in that lookup, use a new one. */ - if (status[0] == NSS_STATUS_SUCCESS && rc6 == 0) - { - tmpbuf4len = 512; - tmpbuf4 = alloca (tmpbuf4len); - } - else - { - tmpbuf4len = tmpbuf6len; - tmpbuf4 = tmpbuf6; - } - /* Next collect IPv4 information. */ while (1) { rc4 = 0; - status[1] = DL_CALL_FCT (fct, (key, AF_INET, &th[1], tmpbuf4, - tmpbuf4len, &rc4, &herrno, + status[1] = DL_CALL_FCT (fct, (key, AF_INET, &th[1], + tmpbuf4.data, tmpbuf4.length, + &rc4, &herrno, ttl == INT32_MAX ? &ttl : NULL, canon == NULL ? &canon : NULL)); if (rc4 != ERANGE || herrno != NETDB_INTERNAL) break; - tmpbuf4 = extend_alloca (tmpbuf4, tmpbuf4len, 2 * tmpbuf4len); + if (!scratch_buffer_grow (&tmpbuf4)) + { + rc4 = ENOMEM; + break; + } } if (rc4 != 0 && herrno == NETDB_INTERNAL) @@ -281,13 +288,11 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, cfct = __nss_lookup_function (nip, "getcanonname_r"); if (cfct != NULL) { - const size_t max_fqdn_len = 256; - char *buf = alloca (max_fqdn_len); char *s; int rc; - if (DL_CALL_FCT (cfct, (key, buf, max_fqdn_len, &s, - &rc, &herrno)) + if (DL_CALL_FCT (cfct, (key, canonbuf.data, canonbuf.length, + &s, &rc, &herrno)) == NSS_STATUS_SUCCESS) canon = s; else @@ -316,18 +321,20 @@ addhstaiX (struct database_dyn *db, int fd, request_header *req, addrfamily = AF_INET6; } - size_t tmpbuflen = 512; - char *tmpbuf = alloca (tmpbuflen); int rc; while (1) { rc = __gethostbyaddr2_r (addr, addrlen, addrfamily, - &hstent_mem, tmpbuf, tmpbuflen, + &hstent_mem, + canonbuf.data, canonbuf.length, &hstent, &herrno, NULL); if (rc != ERANGE || herrno != NETDB_INTERNAL) break; - tmpbuf = extend_alloca (tmpbuf, tmpbuflen, - tmpbuflen * 2); + if (!scratch_buffer_grow (&canonbuf)) + { + rc = ENOMEM; + break; + } } if (rc == 0) @@ -531,6 +538,10 @@ next_nip: dh->usable = false; } + scratch_buffer_free (&tmpbuf6); + scratch_buffer_free (&tmpbuf4); + scratch_buffer_free (&canonbuf); + return timeout; } -- cgit 1.4.1