Re: *alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

Jessica Clarke wrote:
> So it’s not really a case of not being necessary, it’s a case of “are
> you using void * or using void * __capability everywhere to opt into
> capability use?”

Indeed, when I try to compile the current Gnulib code in "hybrid" mode
(CC="clang -march=morello"), I get compilation errors due to mismatches
between 'void *' and 'void * __capability':

../../gltests/../gllib/alignalloc.h:101:11: error: converting capability type 
'void * __capability' to non-capability type 'void *' without an explicit cast; 
if this is intended use __cheri_fromcap
ptr = cheri_bounds_set (ptr, size);
  ^
  (__cheri_fromcap void *)

../../gltests/test-malloca.c:28:24: error: converting capability type 'void * 
__capability' to non-capability type 'void *' without an explicit cast; if this 
is intended use __cheri_fromcap
../../gltests/../gllib/malloca.h:77:8: note: expanded from macro 'malloca'
 ? cheri_bounds_set ((void *) (((uintptr_t)   \
   ^

../../gltests/../gllib/safe-alloc.h:50:11: error: converting capability type 
'void * __capability' to non-capability type 'void *' without an explicit cast; 
if this is intended use __cheri_fromcap
ptr = cheri_bounds_set (ptr, 0);
  ^
  (__cheri_fromcap void *)

> This need to annotate all uses
> (including any third-party library APIs you use with them), which ends
> up spreading throughout entire codebases, is why hybrid is awkward to
> use at scale and only really works in highly-constrained environments.
> For most userspace code there’s very little benefit to using it

I see. So, I'm following your advice:


2023-11-13  Bruno Haible  

Don't use CHERI facilities with CC="clang -march=morello".
Suggested by Jessica Clarke  in
.
* lib/alignalloc.h (alignalloc): Test __CHERI_PURE_CAPABILITY__, not
__CHERI__.
* lib/eealloc.h (eemalloc, eerealloc): Likewise.
* lib/ialloc.h (irealloc, ireallocarray): Likewise.
* lib/malloca.h (malloca): Likewise.
* lib/malloca.c (small_t, mmalloca, freea): Likewise.
* lib/rawmemchr.c (rawmemchr): Likewise.
* lib/safe-alloc.h (safe_alloc_realloc_n): Likewise.
* lib/sigsegv.c (SIGSEGV_FAULT_STACKPOINTER): Likewise.
* lib/ssfmalloc.h (struct dissected_page_header, init_small_block_page,
init_medium_block_page, free_block_from_pool, allocate_block): Likewise.
* tests/test-stdint.c: Likewise.

diff --git a/lib/alignalloc.h b/lib/alignalloc.h
index cb40b344e8..4f75084d13 100644
--- a/lib/alignalloc.h
+++ b/lib/alignalloc.h
@@ -29,7 +29,7 @@
 #include 
 #include 
 #include "idx.h"
-#if defined __CHERI__
+#if defined __CHERI_PURE_CAPABILITY__
 # include 
 #endif
 
@@ -96,7 +96,7 @@ alignalloc (idx_t alignment, idx_t size)
   if (alignment < sizeof (void *))
 alignment = sizeof (void *);
   errno = posix_memalign (, alignment, size | !size);
-#  if defined __CHERI__
+#  if defined __CHERI_PURE_CAPABILITY__
   if (ptr != NULL)
 ptr = cheri_bounds_set (ptr, size);
 #  endif
diff --git a/lib/eealloc.h b/lib/eealloc.h
index bae3915146..1c54465680 100644
--- a/lib/eealloc.h
+++ b/lib/eealloc.h
@@ -36,7 +36,7 @@
 #endif
 
 #include 
-#if defined __CHERI__
+#if defined __CHERI_PURE_CAPABILITY__
 # include 
 #endif
 
@@ -59,7 +59,7 @@ eemalloc (size_t n)
   if (n == 0)
 nx = 1;
   void *ptr = malloc (nx);
-# if defined __CHERI__
+# if defined __CHERI_PURE_CAPABILITY__
   if (ptr != NULL)
 ptr = cheri_bounds_set (ptr, n);
 # endif
@@ -80,7 +80,7 @@ eerealloc (void *p, size_t n)
   if (n == 0)
 nx = 1;
   void *ptr = realloc (p, nx);
-# if defined __CHERI__
+# if defined __CHERI_PURE_CAPABILITY__
   if (ptr != NULL)
 ptr = cheri_bounds_set (ptr, n);
 # endif
diff --git a/lib/ialloc.h b/lib/ialloc.h
index 527b1f48be..5b336ab1c5 100644
--- a/lib/ialloc.h
+++ b/lib/ialloc.h
@@ -29,7 +29,7 @@
 #include 
 #include 
 #include 
-#if defined __CHERI__
+#if defined __CHERI_PURE_CAPABILITY__
 # include 
 #endif
 
@@ -73,7 +73,7 @@ irealloc (void *p, idx_t s)
   /* Work around GNU realloc glitch by treating a zero size as if it
  were 1, so that returning NULL is equivalent to failing.  */
   p = realloc (p, s | !s);
-#if defined __CHERI__
+#if defined __CHERI_PURE_CAPABILITY__
   if (p != NULL)
 p = cheri_bounds_set (p, s);
 #endif
@@ -121,7 +121,7 @@ ireallocarray (void *p, idx_t n, idx_t s)
   if (n == 0 || s == 0)
 nx = sx = 1;
   p = reallocarray (p, nx, sx);
-#if defined __CHERI__
+#if defined __CHERI_PURE_CAPABILITY__
   if (p != NULL && (n == 0 || s == 0))
 p = cheri_bounds_set (p, 0);
 #endif
diff --git a/lib/malloca.c b/lib/malloca.c
index f98fdf152d..5ddb867039 100644
--- a/lib/malloca.c
+++ b/lib/malloca.c
@@ -22,7 +22,7 @@
 #include "malloca.h"
 
 #include 
-#if defined __CHERI__
+#if defined 

Re: *alloc: Take advantage of CHERI bounds-checking

[Paul Eggert]

On 2023-11-12 20:15, Jessica Clarke wrote:

it’s a case of “are
you using void * or using void * __capability everywhere to opt into
capability use?” Unless that’s what you mean by necessary?


It seems pretty clear that Gnulib should run, run away from hybrid mode 
as fast as it can.

Re: *alloc: Take advantage of CHERI bounds-checking

[Paul Eggert]

On 2023-11-12 19:10, Bruno Haible wrote:

So, if I understood it correctly, in hybrid mode (2), programs (especially
memory allocators)_can_  use  and its functions, but it's not
necessary since the programs will also work without it?


So __CHERI__ currently means "either hybrid or pure capability mode" and 
__CHERI_PURE_CAPABILITY__ means "pure capability mode only"? And the 
CHERI folks are thinking of changing __CHERI__ so that it will mean 
"pure capability mode only" in some future version?


It sounds like we should not use __CHERI__, since (a) it doesn't 
currently mean what we think it means and (b) it's not portable to the 
future. And we should change all instances of __CHERI__ to 
__CHERI_PURE_CAPABILITY__, hoping that __CHERI_PURE_CAPABILITY__ will 
continue to mean what it does now.

Re: *alloc: Take advantage of CHERI bounds-checking

[Jessica Clarke]

On 13 Nov 2023, at 03:10, Bruno Haible  wrote:
> 
> Jessica Clarke wrote:
>> I can see in your patches that you're using __CHERI__ as your ABI
>> detection macro. Unfortunately, this currently isn't quite right.
>> ...
>> we also have a hybrid ABI, which is
>> binary-compatible with non-CHERI code, treating all pointers as
>> traditional integer addresses, but with the ability to qualify them with
>> __capability to opt into them being capabilities (and (u)intcap_t for
>> the (u)intptr_t case).
>> ...
>> the
>> __CHERI__ macro is for detecting this latter case, i.e. just that you
>> have CHERI ISA features available (cc -march=morello -mabi=aapcs for
>> Morello)
>> ...
>> What you instead want is the (rather
>> cumbersome) __CHERI_PURE_CAPABILITY__ macro, which is specifically
>> testing for the pure-capability ABI, so hybrid code where pointers are
>> plain integer addresses continues to use the old code paths rather than
>> the new capability ones.
> 
> Thanks for the advice. Indeed, I can see 3 ABIs on cfarm240:
> 
> (1) "clang" — the traditional aarch64 ABI.
> (2) "clang -march=morello" == "clang -march=morello -mabi=aapcs"
>— what you call "hybrid" mode.
> (3) "clang -march=morello -mabi=purecap" == "cc"
> 
> Predefined symbol differences between (1) and (2):
> 
> $ diff <(:|clang -E -dM -|LC_ALL=C sort) <(:|clang -march=morello -E -dM 
> -|LC_ALL=C sort) | grep '^[<>]'
>> #define __ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR__ 256
>> #define __ARM_CAP_PERMISSION_COMPARTMENT_ID__ 128
>> #define __ARM_CAP_PERMISSION_EXECUTIVE__ 2
>> #define __ARM_CAP_PERMISSION_MUTABLE_LOAD__ 64
>> #define __ARM_FEATURE_ATOMICS 1
>> #define __ARM_FEATURE_CRC32 1
>> #define __ARM_FEATURE_DOTPROD 1
>> #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
>> #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
>> #define __ARM_FEATURE_QRDMX 1
>> #define __CHERI_ADDRESS_BITS__ 64
>> #define __CHERI_CAPABILITY_WIDTH__ 128
>> #define __CHERI_CAP_PERMISSION_ACCESS_SYSTEM_REGISTERS__ 512
>> #define __CHERI_CAP_PERMISSION_GLOBAL__ 1
>> #define __CHERI_CAP_PERMISSION_PERMIT_EXECUTE__ 32768
>> #define __CHERI_CAP_PERMISSION_PERMIT_LOAD_CAPABILITY__ 16384
>> #define __CHERI_CAP_PERMISSION_PERMIT_LOAD__ 131072
>> #define __CHERI_CAP_PERMISSION_PERMIT_SEAL__ 2048
>> #define __CHERI_CAP_PERMISSION_PERMIT_STORE_CAPABILITY__ 8192
>> #define __CHERI_CAP_PERMISSION_PERMIT_STORE_LOCAL__ 4096
>> #define __CHERI_CAP_PERMISSION_PERMIT_STORE__ 65536
>> #define __CHERI_CAP_PERMISSION_PERMIT_UNSEAL__ 1024
>> #define __CHERI__ 1
>> #define __INTCAP_MAX__ 9223372036854775807L
>> #define __SIZEOF_CHERI_CAPABILITY__ 16
>> #define __SIZEOF_INTCAP__ 16
>> #define __SIZEOF_UINTCAP__ 16
>> #define __UINTCAP_MAX__ 18446744073709551615UL
> 
> Predefined symbol differences between (2) and (3):
> 
> $ diff <(:|clang -march=morello -E -dM -|LC_ALL=C sort) <(:|clang 
> -march=morello -mabi=purecap -E -dM -|LC_ALL=C sort) | grep '^[<>]'
>> #define __ARM_FEATURE_C64 1
>> #define __CHERI_CAPABILITY_TABLE__ 3
>> #define __CHERI_CAPABILITY_TLS__ 1
>> #define __CHERI_PURE_CAPABILITY__ 2
>> #define __CHERI_SANDBOX__ 4
> < #define __INTPTR_FMTd__ "ld"
> < #define __INTPTR_FMTi__ "li"
>> #define __INTPTR_FMTd__ "Pd"
>> #define __INTPTR_FMTi__ "Pi"
> < #define __INTPTR_TYPE__ long int
> < #define __INTPTR_WIDTH__ 64
>> #define __INTPTR_TYPE__ __intcap
>> #define __INTPTR_WIDTH__ 128
> < #define __POINTER_WIDTH__ 64
>> #define __PIC__ 1
>> #define __POINTER_WIDTH__ 128
> < #define __SIZEOF_POINTER__ 8
>> #define __SIZEOF_POINTER__ 16
> < #define __UINTPTR_FMTX__ "lX"
> < #define __UINTPTR_FMTo__ "lo"
> < #define __UINTPTR_FMTu__ "lu"
> < #define __UINTPTR_FMTx__ "lx"
>> #define __UINTPTR_FMTX__ "PX"
>> #define __UINTPTR_FMTo__ "Po"
>> #define __UINTPTR_FMTu__ "Pu"
>> #define __UINTPTR_FMTx__ "Px"
> < #define __UINTPTR_TYPE__ long unsigned int
> < #define __UINTPTR_WIDTH__ 64
>> #define __UINTPTR_TYPE__ unsigned __intcap
>> #define __UINTPTR_WIDTH__ 128
>> #define __pic__ 1
> 
> So, if I understood it correctly, in hybrid mode (2), programs (especially
> memory allocators) _can_ use  and its functions, but it's not
> necessary since the programs will also work without it?

You can, but they take in a void * __capability, which is not the same
in hybrid as void * (your standard integer). You can cast between the
two with (__cheri_tocap void * __capability)/(__cheri_fromcap void *),
where the former uses DDC (the “default data capability”, null in
purecap code but set to cover the whole address space by default for
hybrid code) as the capability authority and the latter just extracts
the address. But that won’t help your allocator that’s void *(size_t);
you’d have to make it be void * __capability(size_t) and then annotate
every single place those pointers flow with __capability otherwise
you’d go back to dealing with traditional addresses and use DDC’s
bounds for all your loads and stores. This need to annotate all uses
(including any third-party library APIs you use with them), which 

Re: *alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

Jessica Clarke wrote:
> I can see in your patches that you're using __CHERI__ as your ABI
> detection macro. Unfortunately, this currently isn't quite right.
> ...
> we also have a hybrid ABI, which is
> binary-compatible with non-CHERI code, treating all pointers as
> traditional integer addresses, but with the ability to qualify them with
> __capability to opt into them being capabilities (and (u)intcap_t for
> the (u)intptr_t case).
> ...
> the
> __CHERI__ macro is for detecting this latter case, i.e. just that you
> have CHERI ISA features available (cc -march=morello -mabi=aapcs for
> Morello)
> ...
> What you instead want is the (rather
> cumbersome) __CHERI_PURE_CAPABILITY__ macro, which is specifically
> testing for the pure-capability ABI, so hybrid code where pointers are
> plain integer addresses continues to use the old code paths rather than
> the new capability ones.

Thanks for the advice. Indeed, I can see 3 ABIs on cfarm240:

(1) "clang" — the traditional aarch64 ABI.
(2) "clang -march=morello" == "clang -march=morello -mabi=aapcs"
— what you call "hybrid" mode.
(3) "clang -march=morello -mabi=purecap" == "cc"

Predefined symbol differences between (1) and (2):

$ diff <(:|clang -E -dM -|LC_ALL=C sort) <(:|clang -march=morello -E -dM 
-|LC_ALL=C sort) | grep '^[<>]'
> #define __ARM_CAP_PERMISSION_BRANCH_SEALED_PAIR__ 256
> #define __ARM_CAP_PERMISSION_COMPARTMENT_ID__ 128
> #define __ARM_CAP_PERMISSION_EXECUTIVE__ 2
> #define __ARM_CAP_PERMISSION_MUTABLE_LOAD__ 64
> #define __ARM_FEATURE_ATOMICS 1
> #define __ARM_FEATURE_CRC32 1
> #define __ARM_FEATURE_DOTPROD 1
> #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
> #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
> #define __ARM_FEATURE_QRDMX 1
> #define __CHERI_ADDRESS_BITS__ 64
> #define __CHERI_CAPABILITY_WIDTH__ 128
> #define __CHERI_CAP_PERMISSION_ACCESS_SYSTEM_REGISTERS__ 512
> #define __CHERI_CAP_PERMISSION_GLOBAL__ 1
> #define __CHERI_CAP_PERMISSION_PERMIT_EXECUTE__ 32768
> #define __CHERI_CAP_PERMISSION_PERMIT_LOAD_CAPABILITY__ 16384
> #define __CHERI_CAP_PERMISSION_PERMIT_LOAD__ 131072
> #define __CHERI_CAP_PERMISSION_PERMIT_SEAL__ 2048
> #define __CHERI_CAP_PERMISSION_PERMIT_STORE_CAPABILITY__ 8192
> #define __CHERI_CAP_PERMISSION_PERMIT_STORE_LOCAL__ 4096
> #define __CHERI_CAP_PERMISSION_PERMIT_STORE__ 65536
> #define __CHERI_CAP_PERMISSION_PERMIT_UNSEAL__ 1024
> #define __CHERI__ 1
> #define __INTCAP_MAX__ 9223372036854775807L
> #define __SIZEOF_CHERI_CAPABILITY__ 16
> #define __SIZEOF_INTCAP__ 16
> #define __SIZEOF_UINTCAP__ 16
> #define __UINTCAP_MAX__ 18446744073709551615UL

Predefined symbol differences between (2) and (3):

$ diff <(:|clang -march=morello -E -dM -|LC_ALL=C sort) <(:|clang 
-march=morello -mabi=purecap -E -dM -|LC_ALL=C sort) | grep '^[<>]'
> #define __ARM_FEATURE_C64 1
> #define __CHERI_CAPABILITY_TABLE__ 3
> #define __CHERI_CAPABILITY_TLS__ 1
> #define __CHERI_PURE_CAPABILITY__ 2
> #define __CHERI_SANDBOX__ 4
< #define __INTPTR_FMTd__ "ld"
< #define __INTPTR_FMTi__ "li"
> #define __INTPTR_FMTd__ "Pd"
> #define __INTPTR_FMTi__ "Pi"
< #define __INTPTR_TYPE__ long int
< #define __INTPTR_WIDTH__ 64
> #define __INTPTR_TYPE__ __intcap
> #define __INTPTR_WIDTH__ 128
< #define __POINTER_WIDTH__ 64
> #define __PIC__ 1
> #define __POINTER_WIDTH__ 128
< #define __SIZEOF_POINTER__ 8
> #define __SIZEOF_POINTER__ 16
< #define __UINTPTR_FMTX__ "lX"
< #define __UINTPTR_FMTo__ "lo"
< #define __UINTPTR_FMTu__ "lu"
< #define __UINTPTR_FMTx__ "lx"
> #define __UINTPTR_FMTX__ "PX"
> #define __UINTPTR_FMTo__ "Po"
> #define __UINTPTR_FMTu__ "Pu"
> #define __UINTPTR_FMTx__ "Px"
< #define __UINTPTR_TYPE__ long unsigned int
< #define __UINTPTR_WIDTH__ 64
> #define __UINTPTR_TYPE__ unsigned __intcap
> #define __UINTPTR_WIDTH__ 128
> #define __pic__ 1

So, if I understood it correctly, in hybrid mode (2), programs (especially
memory allocators) _can_ use  and its functions, but it's not
necessary since the programs will also work without it?

Bruno

Re: *alloc: Take advantage of CHERI bounds-checking

[Jessica Clarke]

On Sat, Nov 11, 2023 at 08:14:46PM +0100, Bruno Haible wrote:
> I was impressed by the fact that CHERI detected the multithread-safety
> bug of gnulib's use of rand() in the test suite.
> 
> Now I'd like to try CHERI on packages like gettext, and see whether
> it finds bugs that neither valgrind nor the gcc bounds-checking options
> can detect.
> 
> For this purpose, it is useful if all functions that allocate memory
> blocks return bounds for these memory blocks that are as tight as possible.
> malloc(), realloc(), reallocarray(), alloca() already do so.
> (To convince yourself, use a C program that makes use of these functions,
> and print the return values from within gdb. gdb prints pointers with bounds.)
> 
> This set of patches handles most memory allocators that we have in gnulib.
> 
> The API is documented in
> .

Hi Bruno (and Paul, who I see has also posted some CHERI patches),

I can see in your patches that you're using __CHERI__ as your ABI
detection macro. Unfortunately, this currently isn't quite right. CHERI
has two different ways you can compile for it: the first is the purecap
ABI, what people normally mean when they say CHERI, where every pointer
is a capability, but we also have a hybrid ABI, which is
binary-compatible with non-CHERI code, treating all pointers as
traditional integer addresses, but with the ability to qualify them with
__capability to opt into them being capabilities (and (u)intcap_t for
the (u)intptr_t case). Hybrid isn't particularly useful from a security
perspective, it just exists to allow existing non-CHERI code to
interface with CHERI code, but even then it's quite awkward to use, so
our recommendation is to avoid it whenever possible. Unfortunately, the
__CHERI__ macro is for detecting this latter case, i.e. just that you
have CHERI ISA features available (cc -march=morello -mabi=aapcs for
Morello), and so much of this code will now fall over in that
environment (which is in fact the environment that CheriBSD's lib64 uses
so as to enable the use of hybrid code). We don't do a good job of
documenting this (AFAIR we don't document the macros at all in the above
tech report), and ideally __CHERI__ would be the thing that most people
want / expect it to be (one day we likely will make that change), but
for historical reasons it's not. What you instead want is the (rather
cumbersome) __CHERI_PURE_CAPABILITY__ macro, which is specifically
testing for the pure-capability ABI, so hybrid code where pointers are
plain integer addresses continues to use the old code paths rather than
the new capability ones.

Jess

Re: *alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

I wrote:
> Now I'd like to try CHERI on packages like gettext, and see whether
> it finds bugs that neither valgrind nor the gcc bounds-checking options
> can detect.

Did that. Indeed, CHERI found a memory overrun bug that valgrind had not found
[1]. Just by running "make check" after configuring the package with
  CC=cc CFLAGS=-ggdb

Bruno

[1] 
https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=commitdiff;h=c567dde0c0af8bb95b122cd989077b00e23f57e1

Re: *alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

> This set of patches handles most memory allocators that we have in gnulib.

And this patch handles the 'ssfmalloc' memory allocator.


2023-11-11  Bruno Haible  

ssfmalloc: Take advantage of CHERI bounds-checking.
* lib/ssfmalloc.h: Include .
(struct dissected_page_header) [CHERI]: Add field 'whole_page'.
(init_small_block_page, init_medium_block_page) [CHERI]: Initialize it.
(free_block_from_pool) [CHERI]: Use this field to initialize
pool->freeable_page.
(allocate_block) [CHERI]: Return a pointer with a tight upper bound.

>From 131fd99e619b41c007521c97b6566cb833d58fb5 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sun, 12 Nov 2023 00:45:39 +0100
Subject: [PATCH] ssfmalloc: Take advantage of CHERI bounds-checking.

* lib/ssfmalloc.h: Include .
(struct dissected_page_header) [CHERI]: Add field 'whole_page'.
(init_small_block_page, init_medium_block_page) [CHERI]: Initialize it.
(free_block_from_pool) [CHERI]: Use this field to initialize
pool->freeable_page.
(allocate_block) [CHERI]: Return a pointer with a tight upper bound.
---
 ChangeLog   | 10 ++
 lib/ssfmalloc.h | 26 ++
 2 files changed, 36 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index e9cd83b06e..c2a59f8f88 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2023-11-11  Bruno Haible  
+
+	ssfmalloc: Take advantage of CHERI bounds-checking.
+	* lib/ssfmalloc.h: Include .
+	(struct dissected_page_header) [CHERI]: Add field 'whole_page'.
+	(init_small_block_page, init_medium_block_page) [CHERI]: Initialize it.
+	(free_block_from_pool) [CHERI]: Use this field to initialize
+	pool->freeable_page.
+	(allocate_block) [CHERI]: Return a pointer with a tight upper bound.
+
 2023-11-11  Johannes Schindelin  
 
 	vasnprintf: Re-enable parsing of directive with I64 (regr. 2023-03-24).
diff --git a/lib/ssfmalloc.h b/lib/ssfmalloc.h
index 5dd69c8199..14644cb1d4 100644
--- a/lib/ssfmalloc.h
+++ b/lib/ssfmalloc.h
@@ -133,6 +133,9 @@ static void free_block (uintptr_t block);
 #include "thread-optim.h"
 #include "gl_oset.h"
 #include "gl_rbtree_oset.h"
+#ifdef __CHERI__
+# include 
+#endif
 
 /* Help the branch prediction.  */
 #if __GNUC__ >= 3
@@ -178,6 +181,10 @@ struct page_tree_element
 struct dissected_page_header
 {
   struct any_page_header common;
+  #ifdef __CHERI__
+  /* This page, with bounds [page, page + PAGESIZE).  */
+  uintptr_t whole_page;
+  #endif
   /* Amount of free space in this page.  Always a multiple of ALIGNMENT.  */
   pg_offset_t free_space;
   /* The tree element.  */
@@ -386,6 +393,9 @@ init_small_block_page (uintptr_t page)
 {
   struct small_page_header *pageptr = (struct small_page_header *) page;
   pageptr->common.common.page_type = small_page_type;
+  #ifdef __CHERI__
+  pageptr->common.whole_page = page;
+  #endif
 
   /* Initialize available_bitmap.  */
   uint32_t *available_bitmap = small_block_page_available_bitmap (pageptr);
@@ -545,6 +555,9 @@ init_medium_block_page (uintptr_t page)
 {
   struct medium_page_header *pageptr = (struct medium_page_header *) page;
   pageptr->common.common.page_type = medium_page_type;
+  #ifdef __CHERI__
+  pageptr->common.whole_page = page;
+  #endif
   pageptr->num_gaps = 1;
   pageptr->gaps[0].start = MEDIUM_BLOCKS_PAGE_FIRST_GAP_START;
   pageptr->gaps[0].end   = MEDIUM_BLOCKS_PAGE_LAST_GAP_END;
@@ -844,7 +857,11 @@ free_block_from_pool (uintptr_t block, uintptr_t page, struct page_pool *pool)
 FREE_PAGES (pool->freeable_page, PAGESIZE);
 
   /* Don't free the page now, but later.  */
+  #ifdef __CHERI__
+  pool->freeable_page = pageptr->whole_page;
+  #else
   pool->freeable_page = page;
+  #endif
 }
 }
 
@@ -908,6 +925,15 @@ allocate_block (size_t size)
 (size <= SMALL_BLOCK_MAX_SIZE ? _block_pages : _block_pages);
   block = allocate_block_from_pool (size, pool);
   if (mt) gl_lock_unlock (ssfmalloc_lock);
+#if defined __CHERI__
+  if (block != 0)
+{
+  size_t offset = block & (PAGESIZE - 1);
+  block = (uintptr_t) cheri_bounds_set ((void *) (block - offset),
+offset + size)
+  + offset;
+}
+#endif
 }
   return block;
 }
-- 
2.34.1

Re: *alloc: Take advantage of CHERI bounds-checking

[Paul Eggert]

On 2023-11-11 13:23, Bruno Haible wrote:

Ah? Which documented functions should I be wary of?


Not sure. But for starters, vaddr_t is obsolete.

I was toying with the idea of not using cheri.h, and just using compiler 
builtins. That may be going too far, though.

Re: *alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

Paul Eggert wrote:
> I tried hard to add support with as few #ifdefs as possible, to avoid 
> cluttering the code. Instead, I created a relatively small include file 
> ptr-bounds.h that packaged things up into easy-to-use macros. This meant 
> most of the rest of the Emacs code didn't need to use #ifdefs.

It's not clear to me whether the #ifdef approach or the macros approach
are better for maintainability. The #ifdef is clutter, sure, but it tells
the reader of the code "you don't need to worry about this, since it's
not active on your platform". The macros increase the developer's learning
curve, but OTOH are nicer w.r.t. refactorings.

In today's patches, a macro would have been nice in order to avoid a bit
of code duplication in malloca.h. But other than that?

> and CHERI might not survive.

Sure, it's experimental. But even a one-time use of CHERI may allow us
to squash bugs.

> the doc doesn't match the code

Ah? Which documented functions should I be wary of?

Bruno

Re: *alloc: Take advantage of CHERI bounds-checking

[Sam James]

Bruno Haible  writes:

> I was impressed by the fact that CHERI detected the multithread-safety
> bug of gnulib's use of rand() in the test suite.
>
> Now I'd like to try CHERI on packages like gettext, and see whether
> it finds bugs that neither valgrind nor the gcc bounds-checking options
> can detect.
>
> For this purpose, it is useful if all functions that allocate memory
> blocks return bounds for these memory blocks that are as tight as possible.
> malloc(), realloc(), reallocarray(), alloca() already do so.
> (To convince yourself, use a C program that makes use of these functions,
> and print the return values from within gdb. gdb prints pointers with bounds.)
>
> This set of patches handles most memory allocators that we have in gnulib.

Oh, TIL. I didn't realise CHERI provided an API for this. Thank you!

I don't think this applies to gnulib, but it feels relevant enough for
me to mention it: for packages with their own allocator where they
retain a pool, it may be worth adding ASAN attributes/hooks.

Emacs did this a little while ago in
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=cb242bf1514ade34ab93b1db1ea7550093ae5839
to find UAFs where the memory might get reused yet but isn't yet
returned to the underlying malloc/free impl.

>
> The API is documented in
> <https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-947.pdf>.
>
>
> 2023-11-11  Bruno Haible  
>
>   malloca: Take advantage of CHERI bounds-checking.
>   * lib/malloca.h: Include .
>   (malloca) [CHERI]: In the stack-allocation case, return a pointer with
>   a tight lower bound and a tight upper bound.
>   * lib/malloca.c: Include .
>   (small_t) [CHERI]: Define as uintptr_t.
>   (mmalloca) [CHERI]: Return a pointer with a tight upper bound.
>   (freea) [CHERI]: Update.
>
> 2023-11-11  Bruno Haible  
>
>   safe-alloc: Take advantage of CHERI bounds-checking.
>   * lib/safe-alloc.h: Include .
>   (safe_alloc_realloc_n): When count or size is 0, return a pointer whose
>   bounds are of size 0, not 1.
>
> 2023-11-11  Bruno Haible  
>
>   ialloc: Take advantage of CHERI bounds-checking.
>   * lib/ialloc.h: Include .
>   (irealloc): When s is 0, return a pointer whose bounds are of size 0,
>   not 1.
>   (ireallocarray): When n or s is 0, return a pointer whose bounds are of
>   size 0, not 1.
>
> 2023-11-11  Bruno Haible  
>
>   eealloc: Take advantage of CHERI bounds-checking.
>   * lib/eealloc.h: Include .
>   (eemalloc): When n is 0, return a pointer whose bounds are of size 0,
>   not 1.
>   (eerealloc): Likewise.
>
> 2023-11-11  Bruno Haible  
>
>   alignalloc: Take advantage of CHERI bounds-checking.
>   * lib/alignalloc.h: Include .
>   (alignalloc): When size is 0, return a pointer whose bounds are of
>   size 0, not 1.
>
> [2. text/x-patch; 
> 0001-alignalloc-Take-advantage-of-CHERI-bounds-checking.patch]...
>
> [3. text/x-patch; 
> 0002-eealloc-Take-advantage-of-CHERI-bounds-checking.patch]...
>
> [4. text/x-patch; 
> 0003-ialloc-Take-advantage-of-CHERI-bounds-checking.patch]...
>
> [5. text/x-patch; 
> 0004-safe-alloc-Take-advantage-of-CHERI-bounds-checking.patch]...
>
> [6. text/x-patch; 
> 0005-malloca-Take-advantage-of-CHERI-bounds-checking.patch]...

Re: *alloc: Take advantage of CHERI bounds-checking

[Paul Eggert]

On 2023-11-11 11:14, Bruno Haible wrote:

I was impressed by the fact that CHERI detected the multithread-safety
bug of gnulib's use of rand() in the test suite.


I was also impressed in 2017 when Intel MPX found some pointer bugs in 
Emacs, and I added support to Emacs for gcc -fcheck-pointer-bounds, 
which used Intel MPX.


I tried hard to add support with as few #ifdefs as possible, to avoid 
cluttering the code. Instead, I created a relatively small include file 
ptr-bounds.h that packaged things up into easy-to-use macros. This meant 
most of the rest of the Emacs code didn't need to use #ifdefs.


This proved to be beneficial when Intel MPX died. Removing MPX support 
from Emacs was relatively simple:


https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fe2649528b0b7637e6b6851c41e696a1016d8d53

It'd be helpful to do something similar with CHERI, as CHERI is still 
somewhat experimental and mutating and the doc doesn't match the code, 
and CHERI might not survive.

*alloc: Take advantage of CHERI bounds-checking

[Bruno Haible]

I was impressed by the fact that CHERI detected the multithread-safety
bug of gnulib's use of rand() in the test suite.

Now I'd like to try CHERI on packages like gettext, and see whether
it finds bugs that neither valgrind nor the gcc bounds-checking options
can detect.

For this purpose, it is useful if all functions that allocate memory
blocks return bounds for these memory blocks that are as tight as possible.
malloc(), realloc(), reallocarray(), alloca() already do so.
(To convince yourself, use a C program that makes use of these functions,
and print the return values from within gdb. gdb prints pointers with bounds.)

This set of patches handles most memory allocators that we have in gnulib.

The API is documented in
<https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-947.pdf>.


2023-11-11  Bruno Haible  

malloca: Take advantage of CHERI bounds-checking.
* lib/malloca.h: Include .
(malloca) [CHERI]: In the stack-allocation case, return a pointer with
a tight lower bound and a tight upper bound.
* lib/malloca.c: Include .
(small_t) [CHERI]: Define as uintptr_t.
(mmalloca) [CHERI]: Return a pointer with a tight upper bound.
(freea) [CHERI]: Update.

2023-11-11  Bruno Haible  

    safe-alloc: Take advantage of CHERI bounds-checking.
* lib/safe-alloc.h: Include .
(safe_alloc_realloc_n): When count or size is 0, return a pointer whose
bounds are of size 0, not 1.

2023-11-11  Bruno Haible  

ialloc: Take advantage of CHERI bounds-checking.
* lib/ialloc.h: Include .
(irealloc): When s is 0, return a pointer whose bounds are of size 0,
not 1.
(ireallocarray): When n or s is 0, return a pointer whose bounds are of
size 0, not 1.

2023-11-11  Bruno Haible  

eealloc: Take advantage of CHERI bounds-checking.
* lib/eealloc.h: Include .
(eemalloc): When n is 0, return a pointer whose bounds are of size 0,
not 1.
(eerealloc): Likewise.

2023-11-11  Bruno Haible  

alignalloc: Take advantage of CHERI bounds-checking.
* lib/alignalloc.h: Include .
(alignalloc): When size is 0, return a pointer whose bounds are of
size 0, not 1.

>From dba81b1764b575d119fd2ee86f386d461e83be76 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 11 Nov 2023 19:28:26 +0100
Subject: [PATCH 1/5] alignalloc: Take advantage of CHERI bounds-checking.

* lib/alignalloc.h: Include .
(alignalloc): When size is 0, return a pointer whose bounds are of
size 0, not 1.
---
 ChangeLog| 7 +++
 lib/alignalloc.h | 7 +++
 2 files changed, 14 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 3ed337f691..b0f6f9f404 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-11-11  Bruno Haible  
+
+	alignalloc: Take advantage of CHERI bounds-checking.
+	* lib/alignalloc.h: Include .
+	(alignalloc): When size is 0, return a pointer whose bounds are of
+	size 0, not 1.
+
 2023-11-11  Bruno Haible  
 
 	rawmemchr tests: Add test case for last commit.
diff --git a/lib/alignalloc.h b/lib/alignalloc.h
index 6a01aacb4f..cb40b344e8 100644
--- a/lib/alignalloc.h
+++ b/lib/alignalloc.h
@@ -29,6 +29,9 @@
 #include 
 #include 
 #include "idx.h"
+#if defined __CHERI__
+# include 
+#endif
 
 _GL_INLINE_HEADER_BEGIN
 #ifndef ALIGNALLOC_INLINE
@@ -93,6 +96,10 @@ alignalloc (idx_t alignment, idx_t size)
   if (alignment < sizeof (void *))
 alignment = sizeof (void *);
   errno = posix_memalign (, alignment, size | !size);
+#  if defined __CHERI__
+  if (ptr != NULL)
+ptr = cheri_bounds_set (ptr, size);
+#  endif
   return ptr;
 # endif
 }
-- 
2.34.1

>From 4daae72c47a59e20f5312c1febb84e4c187e6acc Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sat, 11 Nov 2023 19:31:56 +0100
Subject: [PATCH 2/5] eealloc: Take advantage of CHERI bounds-checking.

* lib/eealloc.h: Include .
(eemalloc): When n is 0, return a pointer whose bounds are of size 0,
not 1.
(eerealloc): Likewise.
---
 ChangeLog |  8 
 lib/eealloc.h | 23 +++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b0f6f9f404..60aea347a3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2023-11-11  Bruno Haible  
+
+	eealloc: Take advantage of CHERI bounds-checking.
+	* lib/eealloc.h: Include .
+	(eemalloc): When n is 0, return a pointer whose bounds are of size 0,
+	not 1.
+	(eerealloc): Likewise.
+
 2023-11-11  Bruno Haible  
 
 	alignalloc: Take advantage of CHERI bounds-checking.
diff --git a/lib/eealloc.h b/lib/eealloc.h
index f172c6..bae3915146 100644
--- a/lib/eealloc.h
+++ b/lib/eealloc.h
@@ -36,6 +36,9 @@
 #endif
 
 #include 
+#if defined __CHERI__
+# include 
+#endif
 
 _GL_INLINE_HEADER_BEGIN
 #ifndef EEALLOC_INLINE
@@ -52,9 +55,15 @@ EEALLOC_INLINE void *
 eemalloc (size_t n)
 {
   /* If n is zero, allocate a 1-byte block.  */
+  size_t nx = n;