From nobody Sat Jul 01 18:32:56 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QtgmC2gfzz4kdXQ; Sat, 1 Jul 2023 18:32:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QtgmC288Dz4Frv; Sat, 1 Jul 2023 18:32:59 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1688236379; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EXoRJGMrAcl9bwV3EUq4L2R/zv8KTnEte43NqbnUmkI=; b=abJ1ijIJvmu0gb8hU66K71c6st9r6T9i4+Ywpaz23tyVbK2ZyC75d+jhaxGf06T4xy5kXj Pzuc59uprnDmsyQXAuv7Wq0aSNzfiemxlkW3lZ9qIDD+6GnvF5XWOg765excMsOKY5huq5 0u+V/kKQK1CBvoJVeiHkjhCIuPviNE0CkfG1uD9LBJBvAncRd6babIwGl3U267clqSJaGY +N27j4k622XtrgIp3wC0jTgMzVl63QCu6KS1RUEd7CT9btk8FssonDD+qnsCBt/wp84hij SRWXQ88HUtaMJNDjFUvhpr3xWGvvMCnzU66Rw8HJPvN+aJ2JxkS1OpPAy7EiGg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1688236379; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EXoRJGMrAcl9bwV3EUq4L2R/zv8KTnEte43NqbnUmkI=; b=NBdQdNbfT4QPnfhgjharsU9GSvVNt/zVTTkNZePg5NkRW+dMPvis5Ly2MPhw0Wkpp19S+P VYBL/9+4XgyhgHP9v29Vg6v77CswZ4TlE3ilLfstDaZIXaC57yAfKxn3WWHdQlHzJFD6f3 BhuG336A2k5XU+8LsldHRgsS8pv/M6O22tJyQVdryBCRwu0AkR1wIrG2gPZzHNgJGK9vx/ lsJZS+GOFTanB5wyORcb91JwvIH3md1/Iw+aGlHBdgfvOnWxqXJqzAmJn9i90hcoxijz2z o+mwa+RoSdno0CjlQpnJo0XLYrh13gjy8/QynI2cdlCkBWVfVWcuva9luXIMiA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1688236379; a=rsa-sha256; cv=none; b=uiZz+llFc7wGuU19irk+7qfRxEXCMsDDTuIBY5S+CXGa2DLFlVP5KxkfUDZIEHTIhFUJm/ wTvaWIE2MtQZGCZ7URE8Hh1UFNc7v2dUceNM++P6ltuDd8N7M8Y+sJ+o2RQZWb6V8A4O49 q4ScfZsFk9Jum3x3PlVLdfyws3sk8RJB241tDi1iVpWysUMuQoZdIZNPsjvBeh6iUrUkzC O9o/hvZt3dpya33k8Q42gMI508zL29MyrQkNZPgCNGUgXBKslAy1DOFSNj3XmJLBuHlwae OEFGCNHVOfnQUykbWmkG2XbybMCqUvsHW5X96v+gBKH2B7RHoeJvxVwJRlPUeQ== Received: from [IPV6:2601:648:8680:16b0:c9ec:28ca:43a:64ef] (unknown [IPv6:2601:648:8680:16b0:c9ec:28ca:43a:64ef]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4QtgmB5Bw7z19BD; Sat, 1 Jul 2023 18:32:58 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: Date: Sat, 1 Jul 2023 11:32:56 -0700 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: git: 56f3f2d2491e - main - libsecureboot: avoid set but not used errors Content-Language: en-US To: "Simon J. Gerraty" Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org References: <202306300652.35U6qpgP027126@gitrepo.freebsd.org> <2512b2e6-8b57-995f-6901-a1e00a4e9238@FreeBSD.org> <63110.1688159069@kaos.jnpr.net> From: John Baldwin In-Reply-To: <63110.1688159069@kaos.jnpr.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ThisMailContainsUnwantedMimeParts: N On 6/30/23 2:04 PM, Simon J. Gerraty wrote: > John Baldwin wrote: >>> --- >>> lib/libsecureboot/openpgp/opgp_sig.c | 22 ++++++++++++---------- >>> lib/libsecureboot/vets.c | 7 +++++-- >>> 2 files changed, 17 insertions(+), 12 deletions(-) >>> >>> diff --git a/lib/libsecureboot/openpgp/opgp_sig.c b/lib/libsecureboot/openpgp/opgp_sig.c >>> index eec3469e3457..7f4e6fb98fd1 100644 >>> --- a/lib/libsecureboot/openpgp/opgp_sig.c >>> +++ b/lib/libsecureboot/openpgp/opgp_sig.c >>> @@ -464,20 +464,22 @@ verify_asc(const char *sigfile, int flags) >>> size_t n; >>> unsigned char *fdata, *sdata; >>> size_t fbytes, sbytes; >>> - >>> + >>> + fdata = NULL; >>> if ((sdata = read_file(sigfile, &sbytes))) { >>> n = strlcpy(pbuf, sigfile, sizeof(pbuf)); >>> - if ((cp = strrchr(pbuf, '.'))) >>> - *cp = '\0'; >>> - if ((fdata = read_file(pbuf, &fbytes))) { >>> - if (openpgp_verify(pbuf, fdata, fbytes, sdata, >>> - sbytes, flags)) { >>> - free(fdata); >>> - fdata = NULL; >>> + if (n < sizeof(pbuf)) { >>> + if ((cp = strrchr(pbuf, '.'))) >>> + *cp = '\0'; >>> + if ((fdata = read_file(pbuf, &fbytes))) { >>> + if (openpgp_verify(pbuf, fdata, fbytes, sdata, >>> + sbytes, flags)) { >>> + free(fdata); >>> + fdata = NULL; >>> + } >>> } >>> } >>> - } else >>> - fdata = NULL; >>> + } >>> free(sdata); >>> return (fdata); >> >> Most of this change seems to be avoiding reading the "real" file >> if the filename from the signature file was too long to fit into >> pbuf which I think is a different change? > > This is all just levels of paranoia. > strlcpy will truncate the data anyway, but if the buf isn't big enough > to hold the name, someone is playing games and we don't want to play along. Oh that's fine, just didn't seem related to the warning described in the commit log is all. :) >>> diff --git a/lib/libsecureboot/vets.c b/lib/libsecureboot/vets.c >>> index 4375dfa76a89..12191097ff8c 100644 >>> --- a/lib/libsecureboot/vets.c >>> +++ b/lib/libsecureboot/vets.c >>> @@ -241,11 +241,14 @@ x509_cn_get(br_x509_certificate *xc, char *buf, size_t len) >>> mc.vtable->start_cert(&mc.vtable, xc->data_len); >>> mc.vtable->append(&mc.vtable, xc->data, xc->data_len); >>> mc.vtable->end_cert(&mc.vtable); >>> - /* we don' actually care about cert status - just its name */ >>> + /* we don't actually care about cert status - just its name */ >>> err = mc.vtable->end_chain(&mc.vtable); >> >> For cases like this I've removed the variable and used a (void) cast instead to indicate >> that the return value is intentionally unused. > > Right, but I actually want err, so it can be seen in a debugger easily. > It was at least useful when first getting this stuff to work. I'm actually surprised you could see err in the debugger at all in that case since the compiler probably elides it (and associated debug info) entirely. However, one thing you can do is to step inside the function and then use 'finish' which will print the return value after completing the function. If you've just stepped over the function you can also check the relevant register to see the return value (e.g. eax/rax on x86). I think the most recent GDB even has a patch now (possibly controlled by an off by default knob) to print the return value from functions when you are stepping similar to the behavior you get with 'finish'. -- John Baldwin