[libc++][hardening] Classify assertions related to leaks and syscalls. (#77164)

Introduce two new categories:
- `_LIBCPP_ASSERT_VALID_DEALLOCATION`;
- `_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL`.
This commit is contained in:
Konstantin Varlamov 2024-01-22 18:12:58 -08:00 committed by GitHub
parent 8c680451a5
commit 6082478e1a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 59 additions and 18 deletions

View File

@ -291,6 +291,15 @@
// - `_LIBCPP_ASSERT_NON_OVERLAPPING_RANGES` -- for functions that take several ranges as arguments, checks that the
// given ranges do not overlap.
//
// - `_LIBCPP_ASSERT_VALID_DEALLOCATION` -- checks that an attempt to deallocate memory is valid (e.g. the given object
// was allocated by the given allocator). Violating this category typically results in a memory leak.
//
// - `_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL` -- checks that a call to an external API doesn't fail in
// an unexpected manner. This includes triggering documented cases of undefined behavior in an external library (like
// attempting to unlock an unlocked mutex in pthreads). Any API external to the library falls under this category
// (from system calls to compiler intrinsics). We generally don't expect these failures to compromize memory safety or
// otherwise create an immediate security issue.
//
// - `_LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR` -- checks any operations that exchange nodes between containers to make sure
// the containers have compatible allocators.
//
@ -345,8 +354,10 @@ _LIBCPP_HARDENING_MODE_DEBUG
// Overlapping ranges will make algorithms produce incorrect results but don't directly lead to a security
// vulnerability.
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_VALID_DEALLOCATION(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression)
@ -360,6 +371,8 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_VALID_DEALLOCATION(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
@ -376,6 +389,8 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_VALID_DEALLOCATION(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSERT(expression, message)
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message)
@ -391,6 +406,8 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_NON_NULL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_NON_OVERLAPPING_RANGES(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_VALID_DEALLOCATION(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression)
# define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression)

View File

@ -55,7 +55,7 @@ public:
_LIBCPP_HIDE_FROM_ABI constexpr explicit operator bool() const noexcept { return __handle_ != nullptr; }
_LIBCPP_HIDE_FROM_ABI bool done() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "done() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "done() can be called only on suspended coroutines");
return __builtin_coro_done(__handle_);
}
@ -63,13 +63,13 @@ public:
_LIBCPP_HIDE_FROM_ABI void operator()() const { resume(); }
_LIBCPP_HIDE_FROM_ABI void resume() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "resume() can be called only on suspended coroutines");
_LIBCPP_ASSERT_UNCATEGORIZED(!done(), "resume() has undefined behavior when the coroutine is done");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "resume() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(!done(), "resume() has undefined behavior when the coroutine is done");
__builtin_coro_resume(__handle_);
}
_LIBCPP_HIDE_FROM_ABI void destroy() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "destroy() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "destroy() can be called only on suspended coroutines");
__builtin_coro_destroy(__handle_);
}
@ -130,7 +130,7 @@ public:
_LIBCPP_HIDE_FROM_ABI constexpr explicit operator bool() const noexcept { return __handle_ != nullptr; }
_LIBCPP_HIDE_FROM_ABI bool done() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "done() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "done() can be called only on suspended coroutines");
return __builtin_coro_done(__handle_);
}
@ -138,13 +138,13 @@ public:
_LIBCPP_HIDE_FROM_ABI void operator()() const { resume(); }
_LIBCPP_HIDE_FROM_ABI void resume() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "resume() can be called only on suspended coroutines");
_LIBCPP_ASSERT_UNCATEGORIZED(!done(), "resume() has undefined behavior when the coroutine is done");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "resume() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(!done(), "resume() has undefined behavior when the coroutine is done");
__builtin_coro_resume(__handle_);
}
_LIBCPP_HIDE_FROM_ABI void destroy() const {
_LIBCPP_ASSERT_UNCATEGORIZED(__is_suspended(), "destroy() can be called only on suspended coroutines");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(__is_suspended(), "destroy() can be called only on suspended coroutines");
__builtin_coro_destroy(__handle_);
}

View File

@ -68,7 +68,10 @@ public:
}
_LIBCPP_HIDE_FROM_ABI void deallocate(_ValueType* __p, size_t __n) {
_LIBCPP_ASSERT_UNCATEGORIZED(__n <= __max_size(), "deallocate called for size which exceeds max_size()");
_LIBCPP_ASSERT_VALID_DEALLOCATION(
__n <= __max_size(),
"deallocate() called for a size which exceeds max_size(), leading to a memory leak "
"(the argument will overflow and result in too few objects being deleted)");
__res_->deallocate(__p, __n * sizeof(_ValueType), alignof(_ValueType));
}

View File

@ -460,8 +460,21 @@ path __current_path(error_code* ec) {
typedef decltype(&::free) Deleter;
Deleter deleter = &::free;
#else
errno = 0; // Note: POSIX mandates that modifying `errno` is thread-safe.
auto size = ::pathconf(".", _PC_PATH_MAX);
_LIBCPP_ASSERT_UNCATEGORIZED(size >= 0, "pathconf returned a 0 as max size");
if (size == -1) {
if (errno != 0) {
return err.report(capture_errno(), "call to pathconf failed");
// `pathconf` returns `-1` without an error to indicate no limit.
} else {
# if defined(__MVS__) && !defined(PATH_MAX)
size = _XOPEN_PATH_MAX + 1;
# else
size = PATH_MAX + 1;
# endif
}
}
auto buff = unique_ptr<path::value_type[]>(new path::value_type[size + 1]);
path::value_type* ptr = buff.get();
@ -620,7 +633,9 @@ void __permissions(const path& p, perms prms, perm_options opts, error_code* ec)
set_sym_perms = is_symlink(st);
if (m_ec)
return err.report(m_ec);
_LIBCPP_ASSERT_UNCATEGORIZED(st.permissions() != perms::unknown, "Permissions unexpectedly unknown");
// TODO(hardening): double-check this assertion -- it might be a valid (if rare) case when the permissions are
// unknown.
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(st.permissions() != perms::unknown, "Permissions unexpectedly unknown");
if (add_perms)
prms |= st.permissions();
else if (remove_perms)
@ -667,7 +682,7 @@ path __read_symlink(const path& p, error_code* ec) {
detail::SSizeT ret;
if ((ret = detail::readlink(p.c_str(), buff.get(), size)) == -1)
return err.report(capture_errno());
_LIBCPP_ASSERT_UNCATEGORIZED(ret > 0, "TODO");
// Note that `ret` returning `0` would work, resulting in a valid empty string being returned.
if (static_cast<size_t>(ret) >= size)
return err.report(errc::value_too_large);
buff[ret] = 0;

View File

@ -189,7 +189,8 @@ void unsynchronized_pool_resource::__adhoc_pool::__do_deallocate(
return;
}
}
_LIBCPP_ASSERT_UNCATEGORIZED(false, "deallocating a block that was not allocated with this allocator");
// The request to deallocate memory ends up being a no-op, likely resulting in a memory leak.
_LIBCPP_ASSERT_VALID_DEALLOCATION(false, "deallocating a block that was not allocated with this allocator");
}
}

View File

@ -36,7 +36,8 @@ bool mutex::try_lock() noexcept { return __libcpp_mutex_trylock(&__m_); }
void mutex::unlock() noexcept {
int ec = __libcpp_mutex_unlock(&__m_);
(void)ec;
_LIBCPP_ASSERT_UNCATEGORIZED(ec == 0, "call to mutex::unlock failed");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(
ec == 0, "call to mutex::unlock failed. A possible reason is that the mutex wasn't locked");
}
// recursive_mutex
@ -50,7 +51,7 @@ recursive_mutex::recursive_mutex() {
recursive_mutex::~recursive_mutex() {
int e = __libcpp_recursive_mutex_destroy(&__m_);
(void)e;
_LIBCPP_ASSERT_UNCATEGORIZED(e == 0, "call to ~recursive_mutex() failed");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(e == 0, "call to ~recursive_mutex() failed");
}
void recursive_mutex::lock() {
@ -62,7 +63,8 @@ void recursive_mutex::lock() {
void recursive_mutex::unlock() noexcept {
int e = __libcpp_recursive_mutex_unlock(&__m_);
(void)e;
_LIBCPP_ASSERT_UNCATEGORIZED(e == 0, "call to recursive_mutex::unlock() failed");
_LIBCPP_ASSERT_VALID_EXTERNAL_API_CALL(
e == 0, "call to recursive_mutex::unlock() failed. A possible reason is that the mutex wasn't locked");
}
bool recursive_mutex::try_lock() noexcept { return __libcpp_recursive_mutex_trylock(&__m_); }

View File

@ -32,7 +32,10 @@ int main(int, char**) {
const std::size_t maxSize = Traits::max_size(a);
a.deallocate(nullptr, maxSize); // no assertion
TEST_LIBCPP_ASSERT_FAILURE(a.deallocate(nullptr, maxSize + 1), "deallocate called for size which exceeds max_size()");
TEST_LIBCPP_ASSERT_FAILURE(
a.deallocate(nullptr, maxSize + 1),
"deallocate() called for a size which exceeds max_size(), leading to a memory leak "
"(the argument will overflow and result in too few objects being deleted)");
return 0;
}