Date: Fri, 12 Jan 2024 13:25:43 -0800 From: Ravi Pokala <rpokala@freebsd.org> To: Mark Johnston <markj@FreeBSD.org>, <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: <BB320FF2-6B2B-491C-903B-4D8C1F0514C4@panasas.com> In-Reply-To: <202401122112.40CLC7nM039270@gitrepo.freebsd.org> References: <202401122112.40CLC7nM039270@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Mark, > failing to check for errors when copying *in* is a much more severe bug In that you might get panics. But failing to check for errors when copying = *out* can be a security leak; depending on the context, a panic might be pre= ferable. I agree with what Mateusz just said: make __result_use_check contingent on = `clang'. Thanks, Ravi (rpokala@) =EF=BB=BF-----Original Message----- From: <owner-src-committers@freebsd.org <mailto:owner-src-committers@freebs= d.org>> on behalf of Mark Johnston <markj@FreeBSD.org <mailto:markj@FreeBSD.= org>> Date: Friday, January 12, 2024 at 13:12 To: <src-committers@FreeBSD.org <mailto:src-committers@FreeBSD.org>>, <dev-= commits-src-all@FreeBSD.org <mailto:dev-commits-src-all@FreeBSD.org>>, <dev-= commits-src-main@FreeBSD.org <mailto:dev-commits-src-main@FreeBSD.org>> Subject: git: d07acc58d898 - main - systm: Relax __result_use_check annotat= ions The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=3Dd07acc58d8987e8e1205f4a82b77e8= 47ea2d60d3 <https://cgit.FreeBSD.org/src/commit/?id=3Dd07acc58d8987e8e1205f4a8= 2b77e847ea2d60d3> commit d07acc58d8987e8e1205f4a82b77e847ea2d60d3 Author: Mark Johnston <markj@FreeBSD.org <mailto:markj@FreeBSD.org>> AuthorDate: 2024-01-12 15:07:28 +0000 Commit: Mark Johnston <markj@FreeBSD.org <mailto:markj@FreeBSD.org>> 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 <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 u= daddr, 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 ka= ddr, +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 newv= al); 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= ,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BB320FF2-6B2B-491C-903B-4D8C1F0514C4>