From 54f5d8331f28ff4484b6df8ca493b39c34b91af2 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Mon, 31 Jul 2017 21:21:10 -0700 Subject: [PATCH] libc fortify: make socket.h and stdlib.h use diagnose_if Since realpath no longer needs to be overloaded, we can restore the upstream source to purity. We'll be able to do this with most of the other functions when we pull a newer clang in. Bug: 12231437 Test: m checkbuild on bionic internal master; CtsBionicTestCases show no new failures. Change-Id: I484221bba0b291273fece23d2be2f5f9fd713d2c --- libc/include/bits/fortify/socket.h | 78 ++++++++----------- libc/include/bits/fortify/stdlib.h | 16 ++-- libc/include/stdlib.h | 3 +- .../lib/libc/stdlib/realpath.c | 2 +- tests/fortify_compilation_test.cpp | 36 +++++++-- 5 files changed, 70 insertions(+), 65 deletions(-) diff --git a/libc/include/bits/fortify/socket.h b/libc/include/bits/fortify/socket.h index c9e94363f..3e610d6ec 100644 --- a/libc/include/bits/fortify/socket.h +++ b/libc/include/bits/fortify/socket.h @@ -37,64 +37,55 @@ ssize_t __recvfrom_chk(int, void*, size_t, size_t, int, struct sockaddr*, #if defined(__BIONIC_FORTIFY) -#define __recvfrom_bad_size "recvfrom called with size bigger than buffer" -#define __sendto_bad_size "sendto called with size bigger than buffer" +#define __recvfrom_bad_size "'recvfrom' called with size bigger than buffer" +#define __sendto_bad_size "'sendto' called with size bigger than buffer" #if defined(__clang__) #if __ANDROID_API__ >= __ANDROID_API_N__ -__BIONIC_ERROR_FUNCTION_VISIBILITY -ssize_t recvfrom(int fd, void* const buf __pass_object_size0, size_t len, - int flags, struct sockaddr* src_addr, socklen_t* addr_len) - __overloadable - __enable_if(__bos(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && - __bos(buf) < len, "selected when the buffer is too small") - __errorattr(__recvfrom_bad_size); - __BIONIC_FORTIFY_INLINE -ssize_t recvfrom(int fd, void* const buf __pass_object_size0, size_t len, - int flags, struct sockaddr* src_addr, socklen_t* addr_len) - __overloadable { +ssize_t recvfrom(int fd, void* const buf __pass_object_size0, size_t len, int flags, struct sockaddr* src_addr, socklen_t* addr_len) + __overloadable + __clang_error_if(__bos(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos(buf) < len, + __recvfrom_bad_size) { size_t bos = __bos0(buf); if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) { - return __call_bypassing_fortify(recvfrom)(fd, buf, len, flags, src_addr, - addr_len); + return __call_bypassing_fortify(recvfrom)(fd, buf, len, flags, src_addr, addr_len); } - return __recvfrom_chk(fd, buf, len, bos, flags, src_addr, addr_len); } #endif /* __ANDROID_API__ >= __ANDROID_API_N__ */ #if __ANDROID_API__ >= __ANDROID_API_N_MR1__ -__BIONIC_ERROR_FUNCTION_VISIBILITY -ssize_t sendto(int fd, const void* buf, size_t len, int flags, - const struct sockaddr* dest_addr, socklen_t addr_len) - __overloadable - __enable_if(__bos0(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && - __bos0(buf) < len, "selected when the buffer is too small") - __errorattr(__sendto_bad_size); - __BIONIC_FORTIFY_INLINE -ssize_t sendto(int fd, const void* const buf __pass_object_size0, size_t len, - int flags, const struct sockaddr* dest_addr, socklen_t addr_len) - __overloadable { +ssize_t sendto(int fd, const void* const buf __pass_object_size0, size_t len, int flags, const struct sockaddr* dest_addr, socklen_t addr_len) + __overloadable + __clang_error_if(__bos0(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(buf) < len, + __sendto_bad_size) { size_t bos = __bos0(buf); if (bos == __BIONIC_FORTIFY_UNKNOWN_SIZE) { - return __call_bypassing_fortify(sendto)(fd, buf, len, flags, dest_addr, - addr_len); + return __call_bypassing_fortify(sendto)(fd, buf, len, flags, dest_addr, addr_len); } - return __sendto_chk(fd, buf, len, bos, flags, dest_addr, addr_len); } - -__BIONIC_ERROR_FUNCTION_VISIBILITY -ssize_t send(int socket, const void* buf, size_t len, int flags) - __overloadable - __enable_if(__bos0(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && - __bos0(buf) < len, "selected when the buffer is too small") - __errorattr("send called with size bigger than buffer"); #endif /* __ANDROID_API__ >= __ANDROID_API_N_MR1__ */ +__BIONIC_FORTIFY_INLINE +ssize_t recv(int socket, void* const buf __pass_object_size0, size_t len, int flags) + __overloadable + __clang_error_if(__bos0(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(buf) < len, + "'recv' called with size bigger than buffer") { + return recvfrom(socket, buf, len, flags, NULL, 0); +} + +__BIONIC_FORTIFY_INLINE +ssize_t send(int socket, const void* const buf __pass_object_size0, size_t len, int flags) + __overloadable + __clang_error_if(__bos0(buf) != __BIONIC_FORTIFY_UNKNOWN_SIZE && __bos0(buf) < len, + "'send' called with size bigger than buffer") { + return sendto(socket, buf, len, flags, NULL, 0); +} + #else /* defined(__clang__) */ ssize_t __recvfrom_real(int, void*, size_t, int, struct sockaddr*, socklen_t*) __RENAME(recvfrom); __errordecl(__recvfrom_error, __recvfrom_bad_size); @@ -147,20 +138,17 @@ ssize_t sendto(int fd, const void* buf, size_t len, int flags, } #endif /* __ANDROID_API__ >= __ANDROID_API_N_MR1__ */ -#endif /* defined(__clang__) */ -#undef __recvfrom_bad_size -#undef __sendto_bad_size - __BIONIC_FORTIFY_INLINE -ssize_t recv(int socket, void* const buf __pass_object_size0, size_t len, - int flags) __overloadable { +ssize_t recv(int socket, void* buf, size_t len, int flags) { return recvfrom(socket, buf, len, flags, NULL, 0); } __BIONIC_FORTIFY_INLINE -ssize_t send(int socket, const void* const buf __pass_object_size0, size_t len, int flags) - __overloadable { +ssize_t send(int socket, const void* buf, size_t len, int flags) { return sendto(socket, buf, len, flags, NULL, 0); } +#endif /* defined(__clang__) */ +#undef __recvfrom_bad_size +#undef __sendto_bad_size #endif /* __BIONIC_FORTIFY */ diff --git a/libc/include/bits/fortify/stdlib.h b/libc/include/bits/fortify/stdlib.h index bda1d455a..cf4b7eade 100644 --- a/libc/include/bits/fortify/stdlib.h +++ b/libc/include/bits/fortify/stdlib.h @@ -32,22 +32,16 @@ #if defined(__BIONIC_FORTIFY) #define __realpath_buf_too_small_str \ - "realpath output parameter must be NULL or a >= PATH_MAX bytes buffer" + "'realpath' output parameter must be NULL or a pointer to a buffer with >= PATH_MAX bytes" /* PATH_MAX is unavailable without polluting the namespace, but it's always 4096 on Linux */ #define __PATH_MAX 4096 #if defined(__clang__) - -__BIONIC_ERROR_FUNCTION_VISIBILITY -char* realpath(const char* path, char* resolved) __overloadable - __enable_if(__bos(resolved) != __BIONIC_FORTIFY_UNKNOWN_SIZE && - __bos(resolved) < __PATH_MAX, __realpath_buf_too_small_str) - __errorattr(__realpath_buf_too_small_str); - -/* No need for a FORTIFY version; the only things we can catch are at - * compile-time. - */ +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); +/* No need for a definition; the only issues we can catch are at compile-time. */ #else /* defined(__clang__) */ diff --git a/libc/include/stdlib.h b/libc/include/stdlib.h index 55e0fa29e..13c9d3734 100644 --- a/libc/include/stdlib.h +++ b/libc/include/stdlib.h @@ -83,8 +83,7 @@ int atoi(const char*) __attribute_pure__; long atol(const char*) __attribute_pure__; long long atoll(const char*) __attribute_pure__; -char * realpath(const char *path, char *resolved) __overloadable - __RENAME_CLANG(realpath); +char* realpath(const char* path, char* resolved); int system(const char *string); void* bsearch(const void* key, const void* base0, size_t nmemb, size_t size, diff --git a/libc/upstream-freebsd/lib/libc/stdlib/realpath.c b/libc/upstream-freebsd/lib/libc/stdlib/realpath.c index 914ecc9c0..c4bd953e9 100644 --- a/libc/upstream-freebsd/lib/libc/stdlib/realpath.c +++ b/libc/upstream-freebsd/lib/libc/stdlib/realpath.c @@ -48,7 +48,7 @@ __FBSDID("$FreeBSD$"); * in which case the path which caused trouble is left in (resolved). */ char * -realpath(const char * __restrict path, char * __restrict resolved) __overloadable +realpath(const char * __restrict path, char * __restrict resolved) { struct stat sb; char *p, *q, *s; diff --git a/tests/fortify_compilation_test.cpp b/tests/fortify_compilation_test.cpp index bb2b7706c..aab71a125 100644 --- a/tests/fortify_compilation_test.cpp +++ b/tests/fortify_compilation_test.cpp @@ -183,11 +183,20 @@ void test_recvfrom() { sockaddr_in addr; // NOLINTNEXTLINE(whitespace/line_length) - // GCC: error: call to '__recvfrom_error' declared with attribute error: recvfrom called with size bigger than buffer - // CLANG: error: call to unavailable function 'recvfrom': recvfrom called with size bigger than buffer + // GCC: error: call to '__recvfrom_error' declared with attribute error: 'recvfrom' called with size bigger than buffer + // CLANG: error: 'recvfrom' called with size bigger than buffer recvfrom(0, buf, 6, 0, reinterpret_cast(&addr), NULL); } +void test_recv() { + char buf[4] = {0}; + + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__recvfrom_error' declared with attribute error: 'recvfrom' called with size bigger than buffer + // CLANG: error: 'recv' called with size bigger than buffer + recv(0, buf, 6, 0); +} + void test_umask() { // NOLINTNEXTLINE(whitespace/line_length) // GCC: error: call to '__umask_invalid_mode' declared with attribute error: umask called with invalid mode @@ -316,8 +325,8 @@ void test_sendto() { sockaddr_in addr; // NOLINTNEXTLINE(whitespace/line_length) - // GCC: error: call to '__sendto_error' declared with attribute error: sendto called with size bigger than buffer - // CLANG: error: call to unavailable function 'sendto': sendto called with size bigger than buffer + // GCC: error: call to '__sendto_error' declared with attribute error: 'sendto' called with size bigger than buffer + // CLANG: error: 'sendto' called with size bigger than buffer sendto(0, buf, 6, 0, reinterpret_cast(&addr), sizeof(sockaddr_in)); } @@ -325,7 +334,22 @@ void test_send() { char buf[4] = {0}; // NOLINTNEXTLINE(whitespace/line_length) - // GCC: error: call to '__sendto_error' declared with attribute error: sendto called with size bigger than buffer - // CLANG: error: call to unavailable function 'send': send called with size bigger than buffer + // GCC: error: call to '__sendto_error' declared with attribute error: 'sendto' called with size bigger than buffer + // CLANG: error: 'send' called with size bigger than buffer send(0, buf, 6, 0); } + +void test_realpath() { + char buf[4] = {0}; + // NOLINTNEXTLINE(whitespace/line_length) + // GCC: error: call to '__realpath_size_error' declared with attribute error: 'realpath' output parameter must be NULL or a pointer to a buffer with >= PATH_MAX bytes + // NOLINTNEXTLINE(whitespace/line_length) + // CLANG: error: 'realpath' output parameter must be NULL or a pointer to a buffer with >= PATH_MAX bytes + realpath(".", buf); + + // This is fine. + realpath(".", NULL); + + // FIXME: But we should warn on this. + realpath(NULL, buf); +}