summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2015-11-01 20:28:55 +0100
committerLennart Poettering <lennart@poettering.net>2015-11-02 23:07:20 +0100
commit9fe4ea21bec739bfe0ebac5565f0539b0e25b317 (patch)
tree4147053da096c3dacbef1a976b29c9add5c39c95
parent5732a7dbb0efa79cc36c6864a4af2e98685b53d6 (diff)
string-util: rework memory_erase() so that it cannot be optimized away
memory_erase() so far just called memset(), which the compiler might optimize away under certain conditions if it feels there's benefit in it. C11 knows a new memset_s() call that is like memset(), but may not be optimized away. Ideally, we'd just use that call, but glibc currently does not support it. Hence, implement our own simplistic version of it. We use a GCC pragma to turn off optimization for this call, and also use the "volatile" keyword on the pointers to ensure that gcc will use the pointers as-is. According to a variety of internet sources, either one does the trick. However, there are also reports that at least the volatile thing isn't fully correct, hence let's add some snake oil and employ both techniques. https://news.ycombinator.com/item?id=4711346
-rw-r--r--.gitignore1
-rw-r--r--Makefile.am7
-rw-r--r--src/basic/string-util.c31
-rw-r--r--src/basic/string-util.h4
-rw-r--r--src/test/test-string-util.c61
5 files changed, 94 insertions, 10 deletions
diff --git a/.gitignore b/.gitignore
index 7abe8e7dae..1c006b53f8 100644
--- a/.gitignore
+++ b/.gitignore
@@ -256,6 +256,7 @@
/test-socket-util
/test-ssd
/test-strbuf
+/test-string-util
/test-strip-tab-ansi
/test-strv
/test-strxcpyx
diff --git a/Makefile.am b/Makefile.am
index be001bd09e..a601265d78 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1447,6 +1447,7 @@ tests += \
test-utf8 \
test-ellipsize \
test-util \
+ test-string-util \
test-extract-word \
test-parse-util \
test-user-util \
@@ -1748,6 +1749,12 @@ test_util_SOURCES = \
test_util_LDADD = \
libshared.la
+test_string_util_SOURCES = \
+ src/test/test-string-util.c
+
+test_string_util_LDADD = \
+ libshared.la
+
test_extract_word_SOURCES = \
src/test/test-extract-word.c
diff --git a/src/basic/string-util.c b/src/basic/string-util.c
index 63b9b79df9..c3be576816 100644
--- a/src/basic/string-util.c
+++ b/src/basic/string-util.c
@@ -748,23 +748,38 @@ int free_and_strdup(char **p, const char *s) {
return 1;
}
-void string_erase(char *x) {
+#pragma GCC push_options
+#pragma GCC optimize("O0")
+
+void* memory_erase(void *p, size_t l) {
+ volatile uint8_t* x = (volatile uint8_t*) p;
+
+ /* This basically does what memset() does, but hopefully isn't
+ * optimized away by the compiler. One of those days, when
+ * glibc learns memset_s() we should replace this call by
+ * memset_s(), but until then this has to do. */
+
+ for (; l > 0; l--)
+ *(x++) = 'x';
+
+ return p;
+}
+
+#pragma GCC pop_options
+
+char* string_erase(char *x) {
if (!x)
- return;
+ return NULL;
/* A delicious drop of snake-oil! To be called on memory where
* we stored passphrases or so, after we used them. */
- memory_erase(x, strlen(x));
+ return memory_erase(x, strlen(x));
}
char *string_free_erase(char *s) {
- if (!s)
- return NULL;
-
- string_erase(s);
- return mfree(s);
+ return mfree(string_erase(s));
}
bool string_is_safe(const char *p) {
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index 297b8f8232..15244b8184 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -162,8 +162,8 @@ static inline void *memmem_safe(const void *haystack, size_t haystacklen, const
return memmem(haystack, haystacklen, needle, needlelen);
}
-#define memory_erase(p, l) memset((p), 'x', (l))
-void string_erase(char *x);
+void* memory_erase(void *p, size_t l);
+char *string_erase(char *x);
char *string_free_erase(char *s);
DEFINE_TRIVIAL_CLEANUP_FUNC(char *, string_free_erase);
diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c
new file mode 100644
index 0000000000..25444c794a
--- /dev/null
+++ b/src/test/test-string-util.c
@@ -0,0 +1,61 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2015 Lennart Poettering
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "string-util.h"
+
+static void test_string_erase(void) {
+ char *x;
+
+ x = strdupa("");
+ 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"));
+
+ x = strdupa("123456789");
+ assert_se(streq(string_erase(x), "xxxxxxxxx"));
+}
+
+int main(int argc, char *argv[]) {
+ test_string_erase();
+ return 0;
+}