From ca9e3cea6cfdad0d99dcef149cb0ba1d1e1e98f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20H=C3=B6gberg?= Date: Mon, 5 Mar 2018 11:49:34 +0100 Subject: Check the arguments to sys_memcpy and friends Passing NULL is undefined behavior and unconditionally executing these may result in the compiler optimizing away a later NULL check. It can often work since the pointer isn't touched when the length is 0, but it's a major footgun. See ERL-573. --- erts/emulator/beam/sys.h | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/erts/emulator/beam/sys.h b/erts/emulator/beam/sys.h index 290e0b209a..13ae80e4a5 100644 --- a/erts/emulator/beam/sys.h +++ b/erts/emulator/beam/sys.h @@ -996,16 +996,36 @@ erts_refc_read(erts_refc_t *refcp, erts_aint_t min_val) #endif /* #if ERTS_GLB_INLINE_INCL_FUNC_DEF */ -#define sys_memcpy(s1,s2,n) memcpy(s1,s2,n) -#define sys_memmove(s1,s2,n) memmove(s1,s2,n) -#define sys_memcmp(s1,s2,n) memcmp(s1,s2,n) -#define sys_memset(s,c,n) memset(s,c,n) -#define sys_memzero(s, n) memset(s,'\0',n) -#define sys_strcmp(s1,s2) strcmp(s1,s2) -#define sys_strncmp(s1,s2,n) strncmp(s1,s2,n) -#define sys_strcpy(s1,s2) strcpy(s1,s2) -#define sys_strncpy(s1,s2,n) strncpy(s1,s2,n) -#define sys_strlen(s) strlen(s) +/* Thin wrappers around memcpy and friends, which should always be used in + * place of plain memcpy, memset, etc. + * + * Passing NULL to any of these functions is undefined behavior even though it + * may seemingly work when the length (if any) is zero; a compiler can take + * this as a hint that the passed operand may *never* be NULL and then optimize + * based on that information. + * + * (The weird casts in the assertions silence an "always evaluates to true" + * warning when an operand is the address of an lvalue) */ +#define sys_memcpy(s1,s2,n) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), memcpy(s1,s2,n)) +#define sys_memmove(s1,s2,n) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), memmove(s1,s2,n)) +#define sys_memcmp(s1,s2,n) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), memcmp(s1,s2,n)) +#define sys_memset(s,c,n) \ + (ASSERT((void*)(s) != NULL), memset(s,c,n)) +#define sys_memzero(s, n) \ + (ASSERT((void*)(s) != NULL), memset(s,'\0',n)) +#define sys_strcmp(s1,s2) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), strcmp(s1,s2)) +#define sys_strncmp(s1,s2,n) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), strncmp(s1,s2,n)) +#define sys_strcpy(s1,s2) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), strcpy(s1,s2)) +#define sys_strncpy(s1,s2,n) \ + (ASSERT((void*)(s1) != NULL && (void*)(s2) != NULL), strncpy(s1,s2,n)) +#define sys_strlen(s) \ + (ASSERT((void*)(s) != NULL), strlen(s)) /* define function symbols (needed in sys_drv_api) */ #define sys_fp_alloc sys_alloc -- cgit v1.2.3