Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260421123023.GL1827@brightrain.aerifal.cx>
Date: Tue, 21 Apr 2026 08:30:24 -0400
From: Rich Felker <dalias@...c.org>
To: Florian Schmaus <florian.schmaus@...asip.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] qsort: align 'tmp' buffer to preserve CHERI
 capabilities

On Tue, Apr 21, 2026 at 09:57:35AM +0200, Florian Schmaus wrote:
> The cycle() function in qsort.c uses a local stack buffer 'tmp'
> to temporarily hold elements being permuted.
> 
> Under CHERI architectures, like CHERI RISC-V, pointers are represented
> as capabilities, which include an out-of-band hardware tag bit used to
> enforce memory safety. For this tag bit to be preserved when a
> capability is written to memory, the target address must be strictly
> aligned to the capability's size requirement (e.g., 16-byte aligned
> for 128-bit capabilities).
> 
> Previously, 'tmp' was declared as an 'unsigned char' array, which only
> guarantees a 1-byte alignment. If qsort is called to sort an array of
> pointers, writing these capabilities into an unaligned 'tmp' buffer
> causes the processor to strip their validity tags. When these untagged
> capabilities are copied back out and later dereferenced, the hardware
> will throw a capability fault, crashing the program.
> 
> By changing the buffer type to an array of 'void *' we force the
> compiler to align the stack allocation to the architectural
> pointer/capability alignment boundary. This ensures that capabilities
> stored in the buffer retain their tags and remain valid.
> ---
>  src/stdlib/qsort.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/stdlib/qsort.c b/src/stdlib/qsort.c
> index 28607450b885..862a3099d98f 100644
> --- a/src/stdlib/qsort.c
> +++ b/src/stdlib/qsort.c
> @@ -44,7 +44,7 @@ static inline int pntz(size_t p[2]) {
>  
>  static void cycle(size_t width, unsigned char* ar[], int n)
>  {
> -	unsigned char tmp[256];
> +	void* tmp[16 * sizeof(void*)];
>  	size_t l;
>  	int i;
>  
> -- 
> 2.53.0

I'm not 100% sure there's not a way to argue this is valid, but on the
surface it looks like this is introducing UB via effective type rules.
If the goal is just alignment, a union preserving the char array and
just adding a sufficiently aligned member would achieve that without
concern over the type.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.