From nobody Fri Jan 12 22:29:41 2024 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 4TBbnR07fQz56XSc; Fri, 12 Jan 2024 22:29:47 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com [IPv6:2607:f8b0:4864:20::72c]) (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 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TBbnQ5FGGz4myq; Fri, 12 Jan 2024 22:29:46 +0000 (UTC) (envelope-from markjdb@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-qk1-x72c.google.com with SMTP id af79cd13be357-783182d4a09so753177685a.2; Fri, 12 Jan 2024 14:29:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705098585; x=1705703385; darn=freebsd.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:sender :from:to:cc:subject:date:message-id:reply-to; bh=YSqEq0n7Qwe4wVznSViFAQenN05cVskagVGNEIlv8RY=; b=ATnoauQSc7mOEeTfCTzdAgALcNrjzwBaeKv5g0v1Kd27GsGbeoJtekt9cdREa31MjZ 84xO63xc0qrrr6oz0p20OUPCfCKt+jZa3WqP7qoKeT8FfCwS78jI0BU7N0MVTecYGj1x 2Lyp2Y97Ztq1d1iZdQRA0Z6iJKKsRg+edCr0pyewzp9caBG3VY3lAFDfgfiKnrMXGeeo GW7t0cStEa+IId0aLZCQoiHVbrcnEEypJ0DAgEnVF4CKukIhv+RBqhJjzXd8yBT+3vDO 1JlVk/M7NuhUJzVfAY7EgM1FKRTFQhPGFZaFjOUbEBD53WegsFzAm6Vyx1SMedxlzExW NEKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705098585; x=1705703385; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:sender :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YSqEq0n7Qwe4wVznSViFAQenN05cVskagVGNEIlv8RY=; b=stH8v5rAj0NN6jsbX6yU1EqqZ5vLOUv80hbuSjjcNTaiLvqYLwThyBMmK+skdOk77y YPi85tPgdkkJML1cZ3CadgXuyN8yM0f6qGH4ru8/5PlFu7reMt06D16YhMV62NXQkx31 qAk/ueG4ooc/CxSPeEDYqzsMTebAlNP8LgrxWEPMqryhhtSWPYQip/clT4ALGwxR8KLn TspGG2Miqyb2UNAZSQpyGviI5p3X8nLMJxve8Cp7sx1SA5dX182pwywLXhjpS+OYHtP+ LuWbaJ5/iaJcce/0UpDyD4m8oqGGaQSm1090fkU3sNfPguv6dIUjyeMz0hwfD35FsBK3 L3rg== X-Gm-Message-State: AOJu0YyIpk/0NGRd03e79h2/x9cFs9iJf5Y023hpeVirw9bk69melukC gQ4oJ1wKcYv0kveH3zhWo4zLb6S0T0WM7A== X-Google-Smtp-Source: AGHT+IE4OFmHPj5xo21VP6c4KXc4NSXpypHBC72ZFWtTJEUzRiqAAGvYLSj6vX2fjXAJIS1riCGwtw== X-Received: by 2002:a37:ad0c:0:b0:783:28e9:bcb4 with SMTP id f12-20020a37ad0c000000b0078328e9bcb4mr2235171qkm.72.1705098585030; Fri, 12 Jan 2024 14:29:45 -0800 (PST) Received: from nuc (192-0-220-237.cpe.teksavvy.com. [192.0.220.237]) by smtp.gmail.com with ESMTPSA id bq43-20020a05620a46ab00b00783244c7a95sm1361798qkb.24.2024.01.12.14.29.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Jan 2024 14:29:44 -0800 (PST) Date: Fri, 12 Jan 2024 17:29:41 -0500 From: Mark Johnston To: Ravi Pokala Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: d07acc58d898 - main - systm: Relax __result_use_check annotations Message-ID: References: <202401122112.40CLC7nM039270@gitrepo.freebsd.org> 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 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 4TBbnQ5FGGz4myq X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US] On Fri, Jan 12, 2024 at 01:25:43PM -0800, Ravi Pokala wrote: > Hi Mark, > > > failing to check for errors when copying *in* is a much more severe bug > > In that you might get panics. How do you mean? > But failing to check for errors when copying *out* can be a security leak; depending on the context, a panic might be preferable. If something in the kernel fails to check for copyout() errors, userspace will potentially be confused since it expected the kernel to overwrite some memory when it in fact did not. Failing to check for copyin() errors can cause the kernel to potentially end up treating uninitialized (kernel) memory as initialized. That can have security ramifications depending on what the kernel does after. > I agree with what Mateusz just said: make __result_use_check contingent on `clang'. I don't want to do exactly that. Here is my attempt: https://reviews.freebsd.org/D43426 > Thanks, > > Ravi (rpokala@) > > -----Original Message----- > From: > on behalf of Mark Johnston > > Date: Friday, January 12, 2024 at 13:12 > To: >, >, > > Subject: git: d07acc58d898 - main - systm: Relax __result_use_check annotations > > > The branch main has been updated by markj: > > > URL: https://cgit.FreeBSD.org/src/commit/?id=d07acc58d8987e8e1205f4a82b77e847ea2d60d3 > > > commit d07acc58d8987e8e1205f4a82b77e847ea2d60d3 > Author: Mark Johnston > > AuthorDate: 2024-01-12 15:07:28 +0000 > Commit: Mark Johnston > > CommitDate: 2024-01-12 20:56:00 +0000 > > > systm: Relax __result_use_check annotations > > > When compiling with gcc, functions annotated this way can not have their > return values cast away, e.g., with `(void)copyout(...)`. clang permits > it but gcc does not. Since we have a number of such casts for calls > which copy data out of the kernel, and since failing to check for errors > when copying *in* is a much more severe bug, remove some of the > annotations in order to make the gcc build happy. > > > Reviewed by: kib > Reported by: Jenkins > Fixes: 8e36732e6eb5 ("systm: Annotate copyin() and related functions with __result_use_check") > Differential Revision: https://reviews.freebsd.org/D43418 > --- > sys/sys/systm.h | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > > diff --git a/sys/sys/systm.h b/sys/sys/systm.h > index 2da177af91f0..508690cd639e 100644 > --- a/sys/sys/systm.h > +++ b/sys/sys/systm.h > @@ -289,9 +289,9 @@ int __result_use_check copyin(const void * __restrict udaddr, > void * _Nonnull __restrict kaddr, size_t len); > int __result_use_check copyin_nofault(const void * __restrict udaddr, > void * _Nonnull __restrict kaddr, size_t len); > -int __result_use_check copyout(const void * _Nonnull __restrict kaddr, > +int copyout(const void * _Nonnull __restrict kaddr, > void * __restrict udaddr, size_t len); > -int __result_use_check copyout_nofault(const void * _Nonnull __restrict kaddr, > +int copyout_nofault(const void * _Nonnull __restrict kaddr, > void * __restrict udaddr, size_t len); > > > #ifdef SAN_NEEDS_INTERCEPTORS > @@ -313,11 +313,11 @@ int64_t fuword64(volatile const void *base); > int __result_use_check fueword(volatile const void *base, long *val); > int __result_use_check fueword32(volatile const void *base, int32_t *val); > int __result_use_check fueword64(volatile const void *base, int64_t *val); > -int __result_use_check subyte(volatile void *base, int byte); > -int __result_use_check suword(volatile void *base, long word); > -int __result_use_check suword16(volatile void *base, int word); > -int __result_use_check suword32(volatile void *base, int32_t word); > -int __result_use_check suword64(volatile void *base, int64_t word); > +int subyte(volatile void *base, int byte); > +int suword(volatile void *base, long word); > +int suword16(volatile void *base, int word); > +int suword32(volatile void *base, int32_t word); > +int suword64(volatile void *base, int64_t word); > uint32_t casuword32(volatile uint32_t *base, uint32_t oldval, uint32_t newval); > u_long casuword(volatile u_long *p, u_long oldval, u_long newval); > int casueword32(volatile uint32_t *base, uint32_t oldval, uint32_t *oldvalp, > > > >