Re: d07acc58d898 - main - systm: Relax __result_use_check annotations

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 preferable.

I agree with what Mateusz just said: make __result_use_check contingent on `clang'.

Thanks,

Ravi (rpokala@)

-----Original Message-----
From: <owner-src-committers@freebsd.org <mailto:owner-src-committers@freebsd.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 annotations


The branch main has been updated by markj:


URL: https://cgit.FreeBSD.org/src/commit/?id=d07acc58d8987e8e1205f4a82b77e847ea2d60d3 <https://cgit.FreeBSD.org/src/commit/?id=d07acc58d8987e8e1205f4a82b77e847ea2d60d3>


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 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,