From 95bd4884b525ad7a898105863c086113556ada29 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Mon, 14 Aug 2017 14:48:55 -0700 Subject: [PATCH] libc fortify: error on realpath(NULL, foo) I've half a mind to make this a warning instead, since this sort of call isn't UB. That said: - if the user really wants this (I can't imagine why they would), they can just put NULL in a non-const variable, - we're slowly moving to -Werror ~everywhere anyway, and - it's presumably easier to change this from an error to a warning than the other way around Bug: 12231437 Test: m checkbuild on bullhead internal master. No new CtsBionicTestCases failures. Change-Id: Ie8bf5a3455f663686fda4a7450fb35d147fa745e --- libc/include/bits/fortify/stdlib.h | 3 ++- tests/fortify_compilation_test.cpp | 5 +++-- tests/stdlib_test.cpp | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/libc/include/bits/fortify/stdlib.h b/libc/include/bits/fortify/stdlib.h index cf4b7eade..8f3b02c9d 100644 --- a/libc/include/bits/fortify/stdlib.h +++ b/libc/include/bits/fortify/stdlib.h @@ -40,7 +40,8 @@ #if defined(__clang__) char* realpath(const char* path, char* resolved) __clang_error_if(__bos(resolved) != __BIONIC_FORTIFY_UNKNOWN_SIZE && - __bos(resolved) < __PATH_MAX, __realpath_buf_too_small_str); + __bos(resolved) < __PATH_MAX, __realpath_buf_too_small_str) + __clang_error_if(!path, "'realpath': NULL path is never correct; flipped arguments?"); /* No need for a definition; the only issues we can catch are at compile-time. */ #else /* defined(__clang__) */ diff --git a/tests/fortify_compilation_test.cpp b/tests/fortify_compilation_test.cpp index d859ef146..7517fde3d 100644 --- a/tests/fortify_compilation_test.cpp +++ b/tests/fortify_compilation_test.cpp @@ -376,6 +376,7 @@ void test_realpath() { // This is fine. realpath(".", NULL); - // FIXME: But we should warn on this. - realpath(NULL, buf); + char bigbuf[PATH_MAX]; + // CLANG: error: 'realpath': NULL path is never correct; flipped arguments? + realpath(NULL, bigbuf); } diff --git a/tests/stdlib_test.cpp b/tests/stdlib_test.cpp index c724f741f..7d2dc20ce 100644 --- a/tests/stdlib_test.cpp +++ b/tests/stdlib_test.cpp @@ -184,7 +184,9 @@ TEST(stdlib, posix_memalign_overflow) { TEST(stdlib, realpath__NULL_filename) { errno = 0; - char* p = realpath(NULL, NULL); + // Work around the compile-time error generated by FORTIFY here. + const char* path = NULL; + char* p = realpath(path, NULL); ASSERT_TRUE(p == NULL); ASSERT_EQ(EINVAL, errno); }