Re: [musl] [C23 string conversion 1/3] C23: add the new memset_explicit function

[Joakim Sindholt <opensource@zhasha.com>]

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

On Fri, 26 May 2023 11:25:43 +0200, Jens Gustedt <Jens.Gustedt@inria.fr> wrote:
> This function is meant to work around the fact that C compilers are
> allowed to optimize calls to memset out, if they are able to detect
> that the byte array will die soon, anyway. This permission for memset
> may lead to data leak when non-priveledged parts of an application
> would be able to reconstruct secret information from memory received
> through malloc or on the stack.
> 
> This function here is to force compilers to do the clean up operation
> under all circumstances. How to do that is out of the scope of the C
> standard, so there is not much help there, it only describes the
> intent.
> 
> By having a slow bytewise copy, we intent also to have predictable
> timing, such that we can avoid side-channel attacks. We also do our
> best to remove the meta-information, which is the pointer value from
> the stack and combine that with a synchronizing operation at the end.

I don't see how this is in any way useful. It's certainly not part of
the standard, which only says:

> The intention is that the memory store is always performed (i.e.,
> never elided), regardless of optimizations. This is in contrast to
> calls to the memset function (7.26.6.1)

> ---
>  include/string.h             |  1 +
>  src/string/memset_explicit.c | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>  create mode 100644 src/string/memset_explicit.c
> 
> diff --git a/include/string.h b/include/string.h
> index 05019c03..78ccccbd 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -27,6 +27,7 @@ extern "C" {
>  void *memcpy (void *__restrict, const void *__restrict, size_t);
>  void *memmove (void *, const void *, size_t);
>  void *memset (void *, int, size_t);
> +void *memset_explicit(void *, int, size_t);
>  int memcmp (const void *, const void *, size_t);
>  
>  void *(memchr) (const void *, int, size_t);
> diff --git a/src/string/memset_explicit.c b/src/string/memset_explicit.c
> new file mode 100644
> index 00000000..49ced751
> --- /dev/null
> +++ b/src/string/memset_explicit.c
> @@ -0,0 +1,14 @@
> +#include <string.h>
> +#include <stdlib.h>
> +#include <atomic.h>
> +
> +void *memset_explicit(void *dest, register int c, register size_t n)
> +{
> +  register unsigned char volatile *p    = dest;
> +  register unsigned char volatile *stop = p + n;
> +  for (; p < stop; ++p)
> +    *p = c;
> +  // the CAS operation serves as memory barrier, and destroys the
> +  // information, if it happened to be spilled on the stack
> +  return a_cas_p(&dest, dest, 0);
> +}
> -- 
> 2.34.1
> 

Musl effectively already has this function in that it has
explicit_bzero. Why not simply copy it? Hell, while we're at it,
implement explicit_bzero in terms of memset_explicit.

[-- Attachment #2: 0001-implement-C23-memset_explicit.patch --]
[-- Type: application/octet-stream, Size: 1571 bytes --]

From 60a2ad2c4f0b1af8a0c2e693d23e83ce87a6c489 Mon Sep 17 00:00:00 2001
From: Joakim Sindholt <opensource@zhasha.com>
Date: Fri, 26 May 2023 11:50:34 +0200
Subject: [PATCH] implement C23 memset_explicit

---
 include/string.h             | 1 +
 src/string/explicit_bzero.c  | 3 +--
 src/string/memset_explicit.c | 8 ++++++++
 3 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 src/string/memset_explicit.c

diff --git a/include/string.h b/include/string.h
index db73d2a9..de4232b5 100644
--- a/include/string.h
+++ b/include/string.h
@@ -27,6 +27,7 @@ extern "C" {
 void *memcpy (void *__restrict, const void *__restrict, size_t);
 void *memmove (void *, const void *, size_t);
 void *memset (void *, int, size_t);
+void *memset_explicit (void *, int, size_t);
 int memcmp (const void *, const void *, size_t);
 void *memchr (const void *, int, size_t);
 
diff --git a/src/string/explicit_bzero.c b/src/string/explicit_bzero.c
index f2e12f23..738a096b 100644
--- a/src/string/explicit_bzero.c
+++ b/src/string/explicit_bzero.c
@@ -3,6 +3,5 @@
 
 void explicit_bzero(void *d, size_t n)
 {
-	d = memset(d, 0, n);
-	__asm__ __volatile__ ("" : : "r"(d) : "memory");
+	memset_explicit(d, 0, n);
 }
diff --git a/src/string/memset_explicit.c b/src/string/memset_explicit.c
new file mode 100644
index 00000000..ac54f0cf
--- /dev/null
+++ b/src/string/memset_explicit.c
@@ -0,0 +1,8 @@
+#include <string.h>
+
+void *memset_explicit(void *d, int c, size_t n)
+{
+	d = memset(d, c, n);
+	__asm__ __volatile__ ("" : : "r"(d) : "memory");
+	return d;
+}
-- 
2.26.3