From 2d26d8e07ee680995f96597a1cd713dd81491b89 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 5 Feb 2017 20:05:27 -0500 Subject: treewide: replace homegrown memory_erase with explicit_bzero explicit_bzero was added in glibc 2.25. Make use of it. explicit_bzero is hardcoded to zero the memory, so string erase now truncates the string, instead of overwriting it with 'x'. This causes a visible difference only in the journalctl case. --- configure.ac | 4 ++- src/basic/string-util.c | 11 +++---- src/basic/string-util.h | 5 +++- src/reply-password/reply-password.c | 2 +- src/shared/ask-password-api.c | 10 +++---- src/test/test-string-util.c | 35 ++++++++-------------- .../tty-ask-password-agent.c | 4 +-- 7 files changed, 33 insertions(+), 38 deletions(-) diff --git a/configure.ac b/configure.ac index b9143d28ca..ab1d17c531 100644 --- a/configure.ac +++ b/configure.ac @@ -331,13 +331,15 @@ AC_CHECK_DECLS([ kcmp, keyctl, LO_FLAGS_PARTSCAN, - copy_file_range], + copy_file_range, + explicit_bzero], [], [], [[ #include #include #include #include #include +#include #include #include ]]) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 2ba3604ba0..9d2f4bc8f9 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -821,6 +821,7 @@ int free_and_strdup(char **p, const char *s) { return 1; } +#if !HAVE_DECL_EXPLICIT_BZERO /* * Pointer to memset is volatile so that compiler must de-reference * the pointer and can't assume that it points to any function in @@ -831,19 +832,19 @@ typedef void *(*memset_t)(void *,int,size_t); static volatile memset_t memset_func = memset; -void* memory_erase(void *p, size_t l) { - return memset_func(p, 'x', l); +void explicit_bzero(void *p, size_t l) { + memset_func(p, '\0', l); } +#endif char* string_erase(char *x) { - if (!x) return NULL; /* A delicious drop of snake-oil! To be called on memory where * we stored passphrases or so, after we used them. */ - - return memory_erase(x, strlen(x)); + explicit_bzero(x, strlen(x)); + return x; } char *string_free_erase(char *s) { diff --git a/src/basic/string-util.h b/src/basic/string-util.h index e99f7964be..be44dedff4 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -189,7 +189,10 @@ static inline void *memmem_safe(const void *haystack, size_t haystacklen, const return memmem(haystack, haystacklen, needle, needlelen); } -void* memory_erase(void *p, size_t l); +#if !HAVE_DECL_EXPLICIT_BZERO +void explicit_bzero(void *p, size_t l); +#endif + char *string_erase(char *x); char *string_free_erase(char *s); diff --git a/src/reply-password/reply-password.c b/src/reply-password/reply-password.c index 17eab9772e..a17c8a62b8 100644 --- a/src/reply-password/reply-password.c +++ b/src/reply-password/reply-password.c @@ -90,7 +90,7 @@ int main(int argc, char *argv[]) { r = send_on_socket(fd, argv[2], packet, length); finish: - memory_erase(packet, sizeof(packet)); + explicit_bzero(packet, sizeof(packet)); return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 3e877920da..e3b29e390c 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -95,7 +95,7 @@ static int retrieve_key(key_serial_t serial, char ***ret) { if (n < m) break; - memory_erase(p, n); + explicit_bzero(p, n); free(p); m *= 2; } @@ -104,7 +104,7 @@ static int retrieve_key(key_serial_t serial, char ***ret) { if (!l) return -ENOMEM; - memory_erase(p, n); + explicit_bzero(p, n); *ret = l; return 0; @@ -140,7 +140,7 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa return r; serial = add_key("user", keyname, p, n, KEY_SPEC_USER_KEYRING); - memory_erase(p, n); + explicit_bzero(p, n); if (serial == -1) return -errno; @@ -390,7 +390,7 @@ int ask_password_tty( } x = strndup(passphrase, p); - memory_erase(passphrase, p); + explicit_bzero(passphrase, p); if (!x) { r = -ENOMEM; goto finish; @@ -647,7 +647,7 @@ int ask_password_agent( l = strv_new("", NULL); else l = strv_parse_nulstr(passphrase+1, n-1); - memory_erase(passphrase, n); + explicit_bzero(passphrase, n); if (!l) { r = -ENOMEM; goto finish; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index e43373b0f5..4b3e924cfb 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -29,31 +29,20 @@ static void test_string_erase(void) { assert_se(streq(string_erase(x), "")); x = strdupa("1"); - assert_se(streq(string_erase(x), "x")); - - x = strdupa("12"); - assert_se(streq(string_erase(x), "xx")); - - x = strdupa("123"); - assert_se(streq(string_erase(x), "xxx")); - - x = strdupa("1234"); - assert_se(streq(string_erase(x), "xxxx")); - - x = strdupa("12345"); - assert_se(streq(string_erase(x), "xxxxx")); - - x = strdupa("123456"); - assert_se(streq(string_erase(x), "xxxxxx")); - - x = strdupa("1234567"); - assert_se(streq(string_erase(x), "xxxxxxx")); - - x = strdupa("12345678"); - assert_se(streq(string_erase(x), "xxxxxxxx")); + assert_se(streq(string_erase(x), "")); x = strdupa("123456789"); - assert_se(streq(string_erase(x), "xxxxxxxxx")); + assert_se(streq(string_erase(x), "")); + + assert_se(x[1] == '\0'); + assert_se(x[2] == '\0'); + assert_se(x[3] == '\0'); + assert_se(x[4] == '\0'); + assert_se(x[5] == '\0'); + assert_se(x[6] == '\0'); + assert_se(x[7] == '\0'); + assert_se(x[8] == '\0'); + assert_se(x[9] == '\0'); } static void test_ascii_strcasecmp_n(void) { diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index b45490be1a..a17c006d57 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -243,7 +243,7 @@ static int ask_password_plymouth( r = 0; finish: - memory_erase(buffer, sizeof(buffer)); + explicit_bzero(buffer, sizeof(buffer)); return r; } @@ -283,7 +283,7 @@ static int send_passwords(const char *socket_name, char **passwords) { r = log_debug_errno(errno, "sendto(): %m"); finish: - memory_erase(packet, packet_length); + explicit_bzero(packet, packet_length); return r; } -- cgit v1.2.3-54-g00ecf From 1075122f42211ddb319126d6713a68a05056cd9d Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 5 Feb 2017 20:09:41 -0500 Subject: journalctl: replace string_erase with memset('x') The compiler should not be able to optimize out the memset, because optarg is global memory. In this case, not making the argument an empty string is nicer, so just use an open-coded version of string_erase from before the explicit_bzero change. --- src/journal/journalctl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 2639fd6cf5..9ad6f115a1 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -686,7 +686,9 @@ static int parse_argv(int argc, char *argv[]) { r = free_and_strdup(&arg_verify_key, optarg); if (r < 0) return r; - string_erase(optarg); + /* Use memset not string_erase so this doesn't look confusing + * in ps or htop output. */ + memset(optarg, 'x', strlen(optarg)); arg_merge = false; break; -- cgit v1.2.3-54-g00ecf