From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id D44333858C56 for ; Thu, 11 Jul 2024 20:45:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D44333858C56 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 D44333858C56 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720730704; cv=none; b=iF/xl8xlv8UuOGkapnWjQtwmq/aaVmsBz95FMoFRvfIavN13wExPB4NfXotjgCEZ4IHyEgWWaASbkDmo+PJ5x8Y+DnVHRIuQsXv0pSS+pLgZgkA+uakMkbw79BdxPn0uQOSIK/1sQHN4r7HSpomT3QETe+ttVIT4sYQbn24ZpN8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1720730704; c=relaxed/simple; bh=12NiPom5FsHRunDVu4N2FrIAqinfl1Vou3s/Bx7N5uc=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=Ji5YBXBiuDtbLEJW7IOStQPtHeeypoQfjpqByQjLZEgRjQhS/5z1Wlc5OmyhV8zUXJ4/g2vhW1Ua7eUUMgFzhxaxLmljONutk+PNwIxp3CO3FGBDFuKFKmevHdLFaskCPueGqyVJDZBAxotnkmEzI4A+LufBuvOJ2a94pt8yQ+g= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62c.google.com with SMTP id d9443c01a7336-1fb4a807708so12514695ad.2 for ; Thu, 11 Jul 2024 13:45:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1720730701; x=1721335501; 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=7+amuvDXHQMn5s+Ji4ViV4UpHH5c9LomTu8a7eP0JDM=; b=lgam61M2G+KO15GYJVc37kZr6+1Zth2CloMoPjBY4+/rYc7nMwWunHRZB4Nc7dPGDZ 5oc0tDJCZ6bUEv17SlhnDFcybBBPno3veqwt1nRbixC9Swq9Vk4WgvH+CzAAwtl/E+2o G0n1sj3YNesBwwaqhDA5UO8hFdPFqRDCh4Cw5h/GCrjEN5VvR0ro4wqLHG8oRjKQEZqf /4AXHn4kU6mNsqRSVv1tGzh8QoKgsiJbIZxwV2dtSDWtOVBY/Qu6apoj1+5GtsIJ1oJ/ lCt3EcGYJhiqWUidbDhjc6euSc5r9PPWhdpxWpMDXxHxFL8uc1+79jGWNzs12dDHnkBb zv4Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720730701; x=1721335501; 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=7+amuvDXHQMn5s+Ji4ViV4UpHH5c9LomTu8a7eP0JDM=; b=t4rbf6EI5ORQmju2ButEQ/pGQY+gjug1YpTLgY+EjejvZP99r64ya2CiucTPhFWWQR 4DepThT+BpcN03c/AFEj4CaU4FtDbMD3TSyW5aB2V+ddQ0bWKPwhe1N1mgSNvkAMGUQ+ LBvOk/L1asoMEVRXP9pMHoglP+2Oflk7uSUi5KQ7cgaXgEL1lRHy5V4lD5pXvrAVu5mV +HbU2P5VCIx/hzjziTt8iQ090hz2+iZmA9G96KsmHP8YVon9ggysdakIrriGY1a9v/38 kg3xjydPJeUcLyzPxKXUIdkCqHTdyD97guALO4/YUjEt5fh5R6gd3Avn9AS/vEzDx3cI m4Ng== X-Gm-Message-State: AOJu0YxTxFE2iSN7Isd+JjBC8qfSH85DfSeVF9csuenmGDmpM7FCgb8G PjUl8J2tHpWnk/Wbt7nGBMWv+djdaUGeTexRYApmax3zSWBB0XSyf7pAxL6VUuQvOHI6ThMqxEU O X-Google-Smtp-Source: AGHT+IGQuM/HNLMtXin0wz/dnPAGFANn69AuH0XZsaMvqC7Ob51JCAUh0e5jAaFcEqSAUSRMRAdnAg== X-Received: by 2002:a17:902:e750:b0:1fb:3e77:f742 with SMTP id d9443c01a7336-1fbb6d24205mr92183105ad.7.1720730701091; Thu, 11 Jul 2024 13:45:01 -0700 (PDT) Received: from ?IPV6:2804:18:937:f59a:bd25:73f6:c011:293a? ([2804:18:937:f59a:bd25:73f6:c011:293a]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1fbb6ac4012sm54659445ad.225.2024.07.11.13.44.59 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 Jul 2024 13:45:00 -0700 (PDT) Message-ID: <98cbbfab-d718-4e36-95ca-6fae3d1bbff6@linaro.org> Date: Thu, 11 Jul 2024 17:44:58 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] debug: State buffer overflow message more precisely To: libc-alpha@sourceware.org References: <062d77e8-5880-472b-bde6-7804635619f4@linaro.org> Content-Language: en-US From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 11/07/24 17:16, Ingo Blechschmidt wrote: > Dear Adhemerval, > > thank you for your detailed review of my proposed patch! Much > appreciated. > > On Thu Jul 11 11:34:45 2024, Adhemerval Zanella Netto wrote: >> 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. > > I'm not sure I follow. > > In my view, source fortification correctly identified a potentially > problematic call in Privoxy. But closer inspection shows that the call > is actually safe, hence strictly speaking it was a false positive. > > But I wouldn't blame source fortification: It sure looks like a true > positive! &(c->userid) is indeed a pointer to a region of just size 1. > How should source fortification know that, at runtime, the memory > directly proceeding c->userid is allocated as well? > > Privoxy could help source fortification by using a proper variable > length array instead of faking it. But still, I believe the Privoxy code > to be correct and hence the abort to be erroneous. As it's safer to > erroneously abort instead of erroneously continuing, I am all in favor > of the currect practice of aborting the program. I just thought that the > error message could be made more precise.> > Please correct me if I'm mistaken: I got the impression that the purpose > of source fortification is to abort not only in cases where a buffer > overflow would undoubtedly occur, but also in cases where it's very > reasonable to believe that a buffer overflow is about to occur. The problem is once you add UB in the mix, there is no much the compiler can do to help the fortification. The object size passed along on the fortify wrappers through __builtin_object_size/__builtin_dynamic_object_size required well-behave code, if you start to using alias violation to make clever hacks there no much compiler or runtime can do. So sorry, but this patch does not make any sense.