From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc31.google.com (mail-oo1-xc31.google.com [IPv6:2607:f8b0:4864:20::c31]) by sourceware.org (Postfix) with ESMTPS id 59F343858D33 for ; Mon, 28 Aug 2023 13:50:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 59F343858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oo1-xc31.google.com with SMTP id 006d021491bc7-570e63f5224so2178617eaf.0 for ; Mon, 28 Aug 2023 06:50:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1693230629; x=1693835429; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=DS7OTAglcoXhoDAwkFmBTdYKyzmh6ALvq/cZQWhudaY=; b=B77z1wHVmIv8evWZVfWyta5DoCR9kWoD8AqRQpUHByVhhEgUwHcFGX7e7ad+2LNMgp 8lEP0RCZpQE+EwzgkCMqu2LNbopD0ORAlnxdALz5mT3yNMHttM4K+YEGVhNj4SwfQA7M OUtZLbjoEwWoTPzjSN+fuxgc6gYLovt8uPrKZwFnxoKUG1eBtASirUJ/fetQdlECc5n8 nwfMSXmRolmgnJ25Fs/FDRUUy/rEHCQfMh9VXyz2wp1N+kFCqi3cNx+/gwVf9LKGGsDC vcvft3ZH0ZGOByx2HrlsdPtsbvBKxkGJPyTEFpp6jRijgI5nJmA6XguRc6XhYNGFKQb0 raVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693230629; x=1693835429; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DS7OTAglcoXhoDAwkFmBTdYKyzmh6ALvq/cZQWhudaY=; b=ZmAjcngkfiohsTOpA4EUEBXxWPBFGvowQNOc+c12hr21/xS8I5TfGiWqu4mc9r6CYh s9wRBDo5NpaipRW/7yR4tzbGf7eh/pWGQ1QDIrVJVh+w/zVxZVL+0S0zdGpbnacdcm6L r7FOH5Ij7TNXvJm6E3l7PwgUw2Mv9mT5ZQUGSJlvv/VrdffcWxfduCWMOzykMpLQsy5p 8mvchF7qV/1eIpl8urEiU/D9emFwaANHz54eB/L4h0byrHuOofTCni8K9fuOwH/+l4Vl n+DFJLIj6AXlNn3xnVRrVtQ61rYignnDol18qhg2IROcEwruAfcSB5dJUH73YN2tZxKI VGgw== X-Gm-Message-State: AOJu0Yzrv2eHqjcRx/DaB+piGDGSKpIdNplIQo35g/egx0QjdOml//if +WE4HOWvwVQ0nJFMImr6I3fAtnTBRvpmif7/hipfJw== X-Google-Smtp-Source: AGHT+IGmM2dOrvAqNCuM1uhGv0p7IHcPYwsBfo0SFJVcl2NlfH9Au55/4CBIXd3Ibj7QsjSV1mu2MQ== X-Received: by 2002:a05:6870:f6a7:b0:1ba:bef6:9d80 with SMTP id el39-20020a056870f6a700b001babef69d80mr6154535oab.4.1693230628692; Mon, 28 Aug 2023 06:50:28 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c3:578c:9c3a:f97c:ae6e:d589? ([2804:1b3:a7c3:578c:9c3a:f97c:ae6e:d589]) by smtp.gmail.com with ESMTPSA id y19-20020a056870381300b001cce851c7basm4241798oal.11.2023.08.28.06.50.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 28 Aug 2023 06:50:27 -0700 (PDT) Message-ID: Date: Mon, 28 Aug 2023 10:50:21 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH] resolv/res_query: Add note indicating that alloca usage is safe. Content-Language: en-US To: libc-alpha@sourceware.org, Joe Simmons-Talbott References: <20230705181341.1470594-1-josimmon@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20230705181341.1470594-1-josimmon@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.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,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 05/07/23 15:13, Joe Simmons-Talbott via Libc-alpha wrote: > The buffer size is small (< 1024) and fixed sized so alloca is safe > here. > --- > resolv/res_query.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/resolv/res_query.c b/resolv/res_query.c > index 049de91b95..0e0e7be624 100644 > --- a/resolv/res_query.c > +++ b/resolv/res_query.c > @@ -117,6 +117,7 @@ __res_context_query (struct resolv_context *ctx, const char *name, > int n, use_malloc = 0; > > size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE; > + /* alloca is safe here since bufsize < 1024 and fixed sized. */ > u_char *buf = alloca (bufsize); The bufsize on current Linux build is: size_t bufsize = (type == 439963904 ? 2 : 1) * (12 + 4 + 255 + 1); So with upper bound as 544 (2 * (12 + 4 + 255 + 1)). However, it might increase to 2 * PACKETSIZE later with malloc. This is exactly the scenarion scratch_buffer was created, so maybe we should use it. Below a complete untested patch: diff --git a/resolv/res_query.c b/resolv/res_query.c index 049de91b95..b234db83c1 100644 --- a/resolv/res_query.c +++ b/resolv/res_query.c @@ -80,6 +80,7 @@ #include #include #include +#include #if PACKETSZ > 65536 #define MAXPACKET PACKETSZ @@ -114,11 +115,14 @@ __res_context_query (struct resolv_context *ctx, const char *name, struct __res_state *statp = ctx->resp; UHEADER *hp = (UHEADER *) answer; UHEADER *hp2; - int n, use_malloc = 0; - - size_t bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * QUERYSIZE; - u_char *buf = alloca (bufsize); - u_char *query1 = buf; + int n; + + /* It requires 2 times QUERYSIZE for type == T_QUERY_A_AND_AAAA. */ + struct scratch_buffer buf; + scratch_buffer_init (&buf); + _Static_assert (2 * QUERYSIZE <= sizeof (buf.__space.__c), + "scratch_buffer too small"); + u_char *query1 = buf.data; int nquery1 = -1; u_char *query2 = NULL; int nquery2 = 0; @@ -129,14 +133,14 @@ __res_context_query (struct resolv_context *ctx, const char *name, if (type == T_QUERY_A_AND_AAAA) { n = __res_context_mkquery (ctx, QUERY, name, class, T_A, NULL, - query1, bufsize); + query1, buf.length); if (n > 0) { if ((statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0) { /* Use RESOLV_EDNS_BUFFER_SIZE because the receive buffer can be reallocated. */ - n = __res_nopt (ctx, n, query1, bufsize, + n = __res_nopt (ctx, n, query1, buf.length, RESOLV_EDNS_BUFFER_SIZE); if (n < 0) goto unspec_nomem; @@ -146,20 +150,20 @@ __res_context_query (struct resolv_context *ctx, const char *name, /* Align the buffer. */ int npad = ((nquery1 + __alignof__ (HEADER) - 1) & ~(__alignof__ (HEADER) - 1)) - nquery1; - if (n > bufsize - npad) + if (n > buf.length - npad) { n = -1; goto unspec_nomem; } int nused = n + npad; - query2 = buf + nused; + query2 = buf.data + nused; n = __res_context_mkquery (ctx, QUERY, name, class, T_AAAA, - NULL, query2, bufsize - nused); + NULL, query2, buf.length - nused); if (n > 0 && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0) /* Use RESOLV_EDNS_BUFFER_SIZE because the receive buffer can be reallocated. */ - n = __res_nopt (ctx, n, query2, bufsize, + n = __res_nopt (ctx, n, query2, buf.length, RESOLV_EDNS_BUFFER_SIZE); nquery2 = n; } @@ -169,7 +173,7 @@ __res_context_query (struct resolv_context *ctx, const char *name, else { n = __res_context_mkquery (ctx, QUERY, name, class, type, NULL, - query1, bufsize); + query1, buf.length); if (n > 0 && (statp->options & (RES_USE_EDNS0|RES_USE_DNSSEC)) != 0) @@ -181,27 +185,25 @@ __res_context_query (struct resolv_context *ctx, const char *name, advertise = anslen; else advertise = RESOLV_EDNS_BUFFER_SIZE; - n = __res_nopt (ctx, n, query1, bufsize, advertise); + n = __res_nopt (ctx, n, query1, buf.length, advertise); } nquery1 = n; } - if (__glibc_unlikely (n <= 0) && !use_malloc) { + if (__glibc_unlikely (n <= 0)) { /* Retry just in case res_nmkquery failed because of too short buffer. Shouldn't happen. */ - bufsize = (type == T_QUERY_A_AND_AAAA ? 2 : 1) * MAXPACKET; - buf = malloc (bufsize); - if (buf != NULL) { - query1 = buf; - use_malloc = 1; + if (scratch_buffer_set_array_size (&buf, + T_QUERY_A_AND_AAAA ? 2 : 1, + MAXPACKET)) { + query1 = buf.data; goto again; } } if (__glibc_unlikely (n <= 0)) { RES_SET_H_ERRNO(statp, NO_RECOVERY); - if (use_malloc) - free (buf); + scratch_buffer_free (&buf); return (n); } @@ -224,8 +226,7 @@ __res_context_query (struct resolv_context *ctx, const char *name, answerp2_malloced); } - if (use_malloc) - free (buf); + scratch_buffer_free (&buf); if (n < 0) { RES_SET_H_ERRNO(statp, TRY_AGAIN); return (n);