From owner-dev-commits-src-all@freebsd.org Sat Mar 27 18:44:47 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 28A1757CD29; Sat, 27 Mar 2021 18:44:47 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) (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-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4F77730Kfhz4YhG; Sat, 27 Mar 2021 18:44:46 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qv1-xf29.google.com with SMTP id q9so4608903qvm.6; Sat, 27 Mar 2021 11:44:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=ogDpbsCuoPMsgTyQfpDR3D5K4Q8C4zauPcwiE6RPXAQ=; b=Hjb3S+ld1rghDNJdzuEZPVx5eyaJABu6jg6dnsp/WnphFnlMLSBfwpO0DYywZ7S0SS ojp7kN3QgY3Su8idQQz5SfwW04imp2RxnnPgjJ2lwmrpvPO8M2qXrILXGRW306Svqckz enod6myWoxtlQJgsiM/lJXhynT0W5+P01WkXw0TabLdRzhc9FEXJUJSx2jLmSTcT0M5M DE09jaKc2snaS9vG9Mv0YuqBDdM/O2sTUcn+NzbxqTmm3SrRaJCQjvG3rUHj7WFDEheW f8qg8WMogEXSpS0w2I07XvUvjG1Y3N/w9MhmHMAhDH+jaOrh5Auhb75FFZLOnEyYCxPW /1ng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to; bh=ogDpbsCuoPMsgTyQfpDR3D5K4Q8C4zauPcwiE6RPXAQ=; b=VJoFJoyBxYaMc01Q7QEcxYm9XvJnXqV+1N89qugQCS92yahYCEOz1CXUAk5DQKTXPS 88aCIPZbgGmrBcSC9RXygmE30vF6QyK2C0lkclKrKC6aZLy6a8UrJpp1agKVzXobGxUM Aearqt6+qbYkjRz+kNAskq12ZQEAwLBpFkpBx18EvjpQ18t22WXyuq08KBhGvBQIzvEY DxTuOwEuxxEfw8c+E6KpNQ4SSp6TSZzmPPztgQ+0jZdsCatmvmIkP1fhHC7EOyzZJhL1 tNTN6Xf2t5OUffbbxdW9Bu6vvRwUdLjT3NhhUEP4GkU8/EOPesRcQ4sdO6UYdadEqza1 8PaA== X-Gm-Message-State: AOAM530XKC12ROnP1a2urJ0SlresQvfPv6/Yz03rlCAT9JIBaxlHCAzI +OS3l78myI0xKpLVs2xFrws+2efWj4B3Ww== X-Google-Smtp-Source: ABdhPJz7DWkQajekHwKjxSNP7Yb+KK2XvVzFnMRkcGnvsEYQMhIpWBRRd6GIxKqvMnnB3r6cwOabag== X-Received: by 2002:a05:6214:10c7:: with SMTP id r7mr18918723qvs.3.1616870685578; Sat, 27 Mar 2021 11:44:45 -0700 (PDT) Received: from nuc ([142.126.164.150]) by smtp.gmail.com with ESMTPSA id f136sm9525363qke.24.2021.03.27.11.44.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 27 Mar 2021 11:44:45 -0700 (PDT) Sender: Mark Johnston Date: Sat, 27 Mar 2021 14:44:46 -0400 From: Mark Johnston To: Jessica Clarke Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Subject: Re: git: 410556f1f10f - main - libctf: Fix an out-of-bounds read in ctf_lookup_by_name() Message-ID: References: <202103271806.12RI67Bp061468@gitrepo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 4F77730Kfhz4YhG X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 27 Mar 2021 18:44:47 -0000 On Sat, Mar 27, 2021 at 06:16:44PM +0000, Jessica Clarke wrote: > On 27 Mar 2021, at 18:06, Mark Johnston wrote: > > > > The branch main has been updated by markj: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=410556f1f10fd35b350102725fd8504c3cb0afc8 > > > > commit 410556f1f10fd35b350102725fd8504c3cb0afc8 > > Author: Domagoj Stolfa > > AuthorDate: 2021-03-27 18:04:12 +0000 > > Commit: Mark Johnston > > CommitDate: 2021-03-27 18:04:12 +0000 > > > > libctf: Fix an out-of-bounds read in ctf_lookup_by_name() > > > > When prefixes such as struct, union, etc. are compared with the current > > type (e.g. struct foo), a comparison is made with the prefix. The code > > currently assumes that every type is a valid C type with a prefix, > > however at times, garbage ends up in this function causing an > > unpredictable crash with DTrace due to the isspace(*p) call or > > subsequent calls. An example that I've seen of this is the letter 's' > > being passed in, comparing true with struct as the comparison size was > > (q - p) == 1, but then we increment p with the length of "struct", > > resulting in an out of bounds read. > > > > Reviewed by: markj > > MFC after: 1 week > > Differential Revision: https://reviews.freebsd.org/D29435 > > --- > > cddl/contrib/opensolaris/common/ctf/ctf_lookup.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c b/q > > index aa58663309b6..5912cc1a36e8 100644 > > --- a/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c > > +++ b/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c > > @@ -132,8 +132,9 @@ ctf_lookup_by_name(ctf_file_t *fp, const char *name) > > continue; /* skip qualifier keyword */ > > > > for (lp = fp->ctf_lookups; lp->ctl_prefix != NULL; lp++) { > > - if (lp->ctl_prefix[0] == '\0' || > > - strncmp(p, lp->ctl_prefix, (size_t)(q - p)) == 0) { > > + if ((size_t)(q - p) >= lp->ctl_len && > > + (lp->ctl_prefix[0] == '\0' || > > + strncmp(p, lp->ctl_prefix, (size_t)(q - p)) == 0)) { > > for (p += lp->ctl_len; isspace(*p); p++) > > continue; /* skip prefix and next ws */ > > We had a student porting DTrace to CheriBSD as a Master's project notice this > and get this fixed “upstream” in Illumos[1], but neglected to do so in FreeBSD > (and it seems CheriBSD has an earlier version of the patch that I requested be > changed in the upstream review...); you might wish to pull that in instead? > It’s equivalent, just differently formatted, so adds noise to the diff. Ah, I had pondered suggesting the exact same change. I think we can just apply this adjustment as a separate diff. I posted the diff while I rerun the test suite: https://reviews.freebsd.org/D29448 > > Jess > > [1] https://github.com/illumos/illumos-gate/commit/d15d17d4231f87f1571fa6d585377206f360f667 >