From patchwork Sat Oct 9 20:56:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 46035 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4CD61385840A for ; Sat, 9 Oct 2021 20:57:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4CD61385840A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1633813043; bh=otqEGR0MajWoR+QGpLq/XFg9PjM3ZhR63X5ONu5y8sI=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=JFqX0OIyPqR6K1cADCVFaqT5KshiQsQ4XSYOT9GiR6K8N64C2G0cLR1S0UTn+jGlA dF52NnLQtv3JtjZlrCl4QROL57TlwSy6dkTOvqCOcpD3Q3C+L/JtBI5j4IIABMMLoF jQCleV+gRkD9m06HEh2Ibp/y1bZZ1B/gObLgo1ZM= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qt1-x82c.google.com (mail-qt1-x82c.google.com [IPv6:2607:f8b0:4864:20::82c]) by sourceware.org (Postfix) with ESMTPS id B3CCC3858D28 for ; Sat, 9 Oct 2021 20:57:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B3CCC3858D28 Received: by mail-qt1-x82c.google.com with SMTP id b12so4858198qtq.3 for ; Sat, 09 Oct 2021 13:57:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language; bh=otqEGR0MajWoR+QGpLq/XFg9PjM3ZhR63X5ONu5y8sI=; b=nihtaipg/WGUfNZ2Ay1AlnaTKadoHAUnIstaiHQrW8LQQUi9zsvE0Sd0Y+kM29Gb+5 pOQJObmVHzZ2rUn+mwtIWyuUH1GpM5VOQcq0Jw24u1QCa1y4P/c35VE3M1txcbEDXWn4 MPj0Ub6AY8p1ttG6HjimqyiLHCdDLvHNzs63Eih16mMf3+RiyPX/xuKeva1KJlzmZM4v 0UfxET4KK+3iL0ST2cp52JZxfM4XoVq4ytWJ2RE3mmxiI3QuDkyjdzFpEWJEI1U0WFkh nw/0FZWj4/rAbSzztVTF1NaxBtjYEhgyM6X9h7SgJXVgHu9Ycv3wdx1fgmfV1254myB8 FZRQ== X-Gm-Message-State: AOAM531oJCXnSOr1Kpt4I2aGG0v1F8uNVOiRMZpMRMeG+9kvSvVzUnSE OEp4hcdpQKuao9SqWy1LH/XSrR7x8pA= X-Google-Smtp-Source: ABdhPJyt5NX3IB2Pm+7wyqugsXJleuhlkISfAtfqt4+SObYArEOC8nmZYujt6Iuy6juRpofeA+4BYA== X-Received: by 2002:ac8:5981:: with SMTP id e1mr4214152qte.410.1633813021035; Sat, 09 Oct 2021 13:57:01 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-116.hlrn.qwest.net. [184.96.250.116]) by smtp.gmail.com with ESMTPSA id 130sm1967235qkh.99.2021.10.09.13.57.00 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 09 Oct 2021 13:57:00 -0700 (PDT) Subject: [PATCH v2] replace sprintf with strcpy to avoid GCC warning [BZ#28439] To: Florian Weimer , Martin Sebor via Libc-alpha References: <8bc40ccb-9d82-77c9-1277-cbff062b4ce6@gmail.com> <87ily67y46.fsf@mid.deneb.enyo.de> Message-ID: <33640df2-6ba0-5c0c-2829-747f813118b5@gmail.com> Date: Sat, 9 Oct 2021 14:56:59 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <87ily67y46.fsf@mid.deneb.enyo.de> Content-Language: en-US X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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-Patchwork-Original-From: Martin Sebor via Libc-alpha From: Martin Sebor Reply-To: Martin Sebor Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" On 10/9/21 2:16 PM, Florian Weimer wrote: > * Martin Sebor via Libc-alpha: > >> The patch below replaces a call to sprintf with an equivalent >> pair of strcpy calls to avoid a GCC false positive due to >> a recent optimizer improvement (still under review). > > What's the warning? Can we use __snprintf instead? > > The context looks like this: > > char nbuf[MAXDNAME]; > size_t n, d; > > n = strlen(name); > d = strlen(domain); > if (n + d + 1 >= MAXDNAME) { > RES_SET_H_ERRNO(statp, NO_RECOVERY); > return (-1); > } > sprintf(nbuf, "%s.%s", name, domain); > > So it should be possible to use something like this (untested): > > char nbuf[MAXDNAME + 1]; > > /* nbuf[MAXDNAME] is used to detect overlong inputs. */ > nbuf[MAXDNAME] = '\0'; > __snprintf (nbuf, sizeof (nbuf), "%s.%s", name, domain); > if (nbuf[MAXDNAME] != '\0') > { > RES_SET_H_ERRNO(statp, NO_RECOVERY); > return -1; > } > > But I don't know what the warning is about, and if it would still > trigger. The warning is in BZ#28439: res_query.c: In function ‘__res_context_querydomain’: res_query.c:613:35: warning: ‘%s’ directive writing up to 1023 bytes into a region of size between 1 and 1024 [-Wformat-overflow=] 613 | sprintf(nbuf, "%s.%s", name, domain); | ^~ res_query.c:613:17: note: ‘sprintf’ output between 2 and 2048 bytes into a destination of size 1025 613 | sprintf(nbuf, "%s.%s", name, domain); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Your change above suppresses is as well because without the assert GCC knows nothing about the lengths of the source strings. I would have thought the strcpy approach preferable but if you want to use snprintf the recommended way to detect truncation (and avoid -Wformat-truncation) is testing the return value as in the attached patch. Also tested on x86_64-linux. Martin diff --git a/resolv/res_query.c b/resolv/res_query.c index 75b0e5f2f7..adc8a13f75 100644 --- a/resolv/res_query.c +++ b/resolv/res_query.c @@ -589,10 +589,9 @@ __res_context_querydomain (struct resolv_context *ctx, struct __res_state *statp = ctx->resp; char nbuf[MAXDNAME]; const char *longname = nbuf; - size_t n, d; if (domain == NULL) { - n = strlen(name); + size_t n = strlen(name); /* Decrement N prior to checking it against MAXDNAME so that we detect a wrap to SIZE_MAX and return @@ -603,15 +602,13 @@ __res_context_querydomain (struct resolv_context *ctx, return (-1); } longname = name; - } else { - n = strlen(name); - d = strlen(domain); - if (n + d + 1 >= MAXDNAME) { - RES_SET_H_ERRNO(statp, NO_RECOVERY); - return (-1); - } - sprintf(nbuf, "%s.%s", name, domain); } + else if (__snprintf (nbuf, sizeof nbuf, "%s.%s", name, domain) + >= sizeof nbuf) + { + RES_SET_H_ERRNO(statp, NO_RECOVERY); + return -1; + } return __res_context_query (ctx, longname, class, type, answer, anslen, answerp, answerp2, nanswerp2, resplen2, answerp2_malloced);