From fb49d9630dd5468a2935dc521b3273665280c3ca Mon Sep 17 00:00:00 2001
From: Samanta Navarro <ferivoz@riseup.net>
Date: Sat, 28 Nov 2020 11:36:19 +0000
Subject: [PATCH] lib: always clear size in crypt_safe_free

Writing into allocated memory right before calling free can be optimized
away by smart compilers. To prevent this, a volatile access must be
performed. This happens already in crypt_safe_memzero.

It was difficult to provoke GCC to remove the assignment, but I was able
to find a way to prove the theory:

* Build cryptsetup with: CFLAGS="-flto -O3 -g" ./configure --enable-static
* Create main.c:

#include <libcryptsetup.h>

int
main(void) {
        char *x = crypt_safe_alloc(64);
        crypt_safe_free(x);
        return 0;
}

* Build the program with: gcc -O3 -flto -static -o main main.c -lcryptsetup
* Disassemble: objdump -d main

My output on an amd64 system is:

0000000000401670 <main>:
  401670:       41 54                   push   %r12
  401672:       bf f0 03 00 00          mov    $0x3f0,%edi
  401677:       55                      push   %rbp
  401678:       48 83 ec 08             sub    $0x8,%rsp
  40167c:       e8 ff 4d 01 00          callq  416480 <__libc_malloc>
  401681:       48 85 c0                test   %rax,%rax
  401684:       74 2f                   je     4016b5 <main+0x45>
  401686:       48 c7 00 e8 03 00 00    movq   $0x3e8,(%rax)
  40168d:       4c 8d 60 08             lea    0x8(%rax),%r12
  401691:       48 89 c5                mov    %rax,%rbp
  401694:       be e8 03 00 00          mov    $0x3e8,%esi
  401699:       4c 89 e7                mov    %r12,%rdi
  40169c:       e8 4f 76 01 00          callq  418cf0 <explicit_bzero>
  4016a1:       48 8b 75 00             mov    0x0(%rbp),%rsi
  4016a5:       4c 89 e7                mov    %r12,%rdi
  4016a8:       e8 43 76 01 00          callq  418cf0 <explicit_bzero>
  4016ad:       48 89 ef                mov    %rbp,%rdi
  4016b0:       e8 3b 54 01 00          callq  416af0 <__free>
  4016b5:       48 83 c4 08             add    $0x8,%rsp
  4016b9:       31 c0                   xor    %eax,%eax
  4016bb:       5d                      pop    %rbp
  4016bc:       41 5c                   pop    %r12
  4016be:       c3                      retq
  4016bf:       90                      nop

You can see that the memory allocation and explicit_bzero calls were not
optimized away. But the size assignment disappeared.

Compiling without -O3 or without -flto does not inline the calls and
keeps the assignment. Also the shared library shipped with my
distribution has the assignment.
---
 lib/utils_safe_memory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/utils_safe_memory.c b/lib/utils_safe_memory.c
index 8c1fb5cb..6908ea42 100644
--- a/lib/utils_safe_memory.c
+++ b/lib/utils_safe_memory.c
@@ -66,6 +66,7 @@ void *crypt_safe_alloc(size_t size)
 void crypt_safe_free(void *data)
 {
 	struct safe_allocation *alloc;
+	volatile size_t *s;
 
 	if (!data)
 		return;
@@ -75,7 +76,8 @@ void crypt_safe_free(void *data)
 
 	crypt_safe_memzero(data, alloc->size);
 
-	alloc->size = 0x55aa55aa;
+	s = (volatile size_t *)&alloc->size;
+	*s = 0x55aa55aa;
 	free(alloc);
 }