From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <SRS0=UpP3=OL=linaro.org=adhemerval.zanella@sourceware.org>
Received: from mail-pf1-x430.google.com (mail-pf1-x430.google.com [IPv6:2607:f8b0:4864:20::430])
by sourceware.org (Postfix) with ESMTPS id 6D5BA3844040
for <libc-alpha@sourceware.org>; Thu, 11 Jul 2024 14:34:49 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6D5BA3844040
Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org
ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6D5BA3844040
Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::430
ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720708491; cv=none;
b=rZSxATk5CBvybtwjfGO0IJRyvPt48hui2Z1xyqps8ysQFX3S9Kzw1EXgjlw3Q5hZiJ5FjJupVScPZPgzIerNPDmq+0o+nx4/jedtZteoMXtbOQnfud2B4rl6HrbazqcAJq+VBlJ0+xM42Y2WGO7KMw26bMHhzyz91BwYUh7qLew=
ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key;
t=1720708491; c=relaxed/simple;
bh=c1K7DGZ40T3q/xYUszpottSjUeU9qitNo/lB0t/CixY=;
h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=LudCGZLk9sxj2US2zJeJisOmGwZjevECg4YpkoTZ6cmX4SDAj//4ppqF30FNlH6otPPb4a36GEP622Qm7YG/jmtsVn/rx3wRTYSBolfk3xw9+EPyQIUxTXvLnLsnDLpjXDAruf4FC13e+yVn7cePFK9aUVp3dVAXizvyH3fQq54=
ARC-Authentication-Results: i=1; server2.sourceware.org
Received: by mail-pf1-x430.google.com with SMTP id d2e1a72fcca58-70b0013cf33so906731b3a.2
for <libc-alpha@sourceware.org>; Thu, 11 Jul 2024 07:34:49 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=linaro.org; s=google; t=1720708488; x=1721313288; darn=sourceware.org;
h=content-transfer-encoding:in-reply-to:organization:from
:content-language:references:to:subject:user-agent:mime-version:date
:message-id:from:to:cc:subject:date:message-id:reply-to;
bh=gpyRPhSotGyO7Ovl3GyUJ/KnDimEsLEi0vyXNdX7ArI=;
b=rRn7a9YevZDkmoRaNE6bwtApyXBV9rATcb6unx1OgWJmU429yjLdXdKkNZreuViii0
IuNcRWQW//g0v8z1/zCzQ071dcjgS2Ma+imyqWfZWWcUIpwSGnzoKOLm5Mhwllk31SYX
kFTuiws5WgXRNNAGLi3CUZ15cIfm26ZuF99RWsqyAjQp34FexRXAiDVGLIVUa/mfHTsr
Oz3/rp71Tkt9b8hXen9Kjyo6OUqY2x8wFGqQsXq27KbVfgsAtY4BstKyWFwc4SC/w+C5
hTVQVnMg9QNiVzVS1gOFMZCcOfdzGpbmflrj/MVdpKyFQJQvtbnO+aRyvP1CizMbdYFj
903A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1720708488; x=1721313288;
h=content-transfer-encoding:in-reply-to:organization:from
:content-language:references:to:subject:user-agent:mime-version:date
:message-id:x-gm-message-state:from:to:cc:subject:date:message-id
:reply-to;
bh=gpyRPhSotGyO7Ovl3GyUJ/KnDimEsLEi0vyXNdX7ArI=;
b=U1RLwiPfTtzQWvmosNZPtCBdkwZdT7Y/JXQhW9zNYAJz4V6S+TOUbzBF3i4KlWVYen
BcqDDEeaG85Ec3MYEKBlNf5ygxYYDx0YME0Zf0vXIlV8OIWoZh2bnyr8OyCo8rPM1KB3
zfT69TPiyHfELnFl4X8/UsFTMxao3dYd3b0mL/0tnzWl2xyQzxCG3zaHt53hbbceaLQe
6nfcp4vbCJSpu0oKv0lzWQsufoW9Qqu0nWb6IuRGYwimRagtuqoAtg0AFaptg4thu477
vBGnCgrIPmGHQKlhfp140D2TpPezmo0BM8x9aQKcURknQExXCESmhJ2FEt/51Ltp4OPP
EDuA==
X-Forwarded-Encrypted: i=1; AJvYcCUBznrO4my/e665oHPNK2xRgPORTLTbQZqVTRv4txQk0sUQksTlmbkWgghBp0DhBNwBCyxz0MwzZK+IA26YHgCUQ+aBycTRA4FZ
X-Gm-Message-State: AOJu0YxhbS4Hq9mHAjgFy5x3KXh+dOp9XCntGSzNvxZ5tUw2ns/LuUBi
ZyL1OyUrFwhXE0UMeji1mVFCwB1z3KUxLdIVErgfOsuRT89Wp23QNqIA4SPPvML2YltynNmrAsI
5
X-Google-Smtp-Source: AGHT+IE31krLNyptLz+w/nc17FN+QWNjWkshwVF8d991pUW5nYRbf6JJa/Yz4qftu9+jOrTJpglIbw==
X-Received: by 2002:a05:6a21:6da6:b0:1bd:2214:e92f with SMTP id adf61e73a8af0-1c29821ffbdmr9022970637.14.1720708488346;
Thu, 11 Jul 2024 07:34:48 -0700 (PDT)
Received: from ?IPV6:2804:18:16a:ab94:b941:2bd3:9938:7945? ([2804:18:16a:ab94:b941:2bd3:9938:7945])
by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fbb6a2b72asm51181875ad.82.2024.07.11.07.34.46
(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);
Thu, 11 Jul 2024 07:34:47 -0700 (PDT)
Message-ID: <062d77e8-5880-472b-bde6-7804635619f4@linaro.org>
Date: Thu, 11 Jul 2024 11:34:45 -0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] debug: State buffer overflow message more precisely
To: Ingo Blechschmidt <iblech@speicherleck.de>, libc-alpha@sourceware.org
References: <ZorA48gZxSHgPttS@quasitopos>
Content-Language: en-US
From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Organization: Linaro
In-Reply-To: <ZorA48gZxSHgPttS@quasitopos>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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: <libc-alpha.sourceware.org>
On 07/07/24 13:23, Ingo Blechschmidt wrote:
> This patch contributes to our source fortification endeavors by
> slightly rewording the error message on detecting a probable buffer
> overflow. The new error message adds a bit of nuance, helping to
> demarcate the efforts of source fortification from an unmitigated
> actual buffer overflow, thereby describing the situation more precisely,
> preventing misunderstandings and highlighting source fortification.
>
> I ran into this issue when debugging a problem with Privoxy
> (https://github.com/NixOS/nixpkgs/issues/265654). Source fortification
> correctly identified a libc call which was potentially problematic and
> (in my view) misleading, but actually safe. A more precise error
> message, as proposed by this patch, would have sped up the diagnosis.
>From the bug report, the code does seems to be issuing UB and compiler itself
warns about. With a reduced testcase:
---
$ cat t.c
#define _GNU_SOURCE
#include <string.h>
#define BUFFER_SIZE 5000
static const char socks_userid[] = "anonymous";
struct socks_op {
unsigned char vn; /* socks version number */
unsigned char cd; /* command code */
unsigned char dstport[2]; /* destination port */
unsigned char dstip[4]; /* destination address */
char userid; /* first byte of userid */
char padding[3]; /* make sure sizeof(struct socks_op) is endian-independent. */
/* more bytes of the userid follow, terminated by a NULL */
};
void foo (void)
{
char buf[BUFFER_SIZE];
struct socks_op *c = (struct socks_op *)buf;
strlcpy(&(c->userid), socks_userid, sizeof(buf) - sizeof(struct socks_op));
}
$ gcc -Wall t.c -c -O2 -D_FORTIFY_SOURCE=2
t.c: In function ‘foo’:
t.c:22:3: warning: ‘strlcpy’ writing 4988 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]
22 | strlcpy(&(c->userid), socks_userid, sizeof(buf) - sizeof(struct socks_op));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
t.c:13:9: note: destination object ‘userid’ of size 1
13 | char userid; /* first byte of userid */
| ^~~~~~
In file included from /usr/include/features.h:502,
from /usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33,
from /usr/include/string.h:26,
from t.c:2:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:150:1: note: in a call to function ‘strlcpy’ declared with attribute ‘access (write_only, 1, 3)’
150 | __NTH (strlcpy (char *__restrict __dest, const char *__restrict __src,
| ^~~~~
---
And I don't think changing the error message improves anything here,
if __chk_fail is called the program will be terminated anyway. If you
think this is a false-positive, we will need to check whether the compiler
expansion for the strlcpy is being correct (meaning that __glibc_objsize
is passing the expected values), not to paper over the log.
> ---
> debug/chk_fail.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/debug/chk_fail.c b/debug/chk_fail.c
> index 77d54c6706..44f367deec 100644
> --- a/debug/chk_fail.c
> +++ b/debug/chk_fail.c
> @@ -25,6 +25,6 @@ void
> __attribute__ ((noreturn))
> __chk_fail (void)
> {
> - __fortify_fail ("buffer overflow detected");
> + __fortify_fail ("probable buffer overflow detected");
> }
> libc_hidden_def (__chk_fail)