Skip to content

Commit 9f3d083

Browse files
authored
[win/asan] Ensure errno gets set correctly for strtol (llvm#109258)
This fixes two problems with asan's interception of `strtol` on Windows: 1. In the dynamic runtime, the `strtol` interceptor calls out to ntdll's `strtol` to perform the string conversion. Unfortunately, that function doesn't set `errno`. This has been a long-standing problem (llvm#34485), but it was not an issue when using the static runtime. After the static runtime was removed recently (llvm#107899), the problem became more urgent. 2. A module linked against the static CRT will have a different instance of `errno` than the ASan runtime, since that's now always linked against the dynamic CRT. That means even if the ASan runtime sets `errno` correctly, the calling module will not see it. This patch fixes the first problem by making the `strtol` interceptor call out to `strtoll` instead, and do 32-bit range checks on the result. I can't think of any reasonable way to fix the second problem, so we should stop intercepting `strtol` in the static runtime thunk. I checked the list of functions in the thunk, and `strtol` and `strtoll` are the only ones that set `errno`. (`strtoll` was already missing, probably by mistake.)
1 parent d84411f commit 9f3d083

File tree

6 files changed

+59
-2
lines changed

6 files changed

+59
-2
lines changed

compiler-rt/lib/asan/asan_interceptors.cpp

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -650,9 +650,34 @@ static ALWAYS_INLINE auto StrtolImpl(void *ctx, Fn real, const char *nptr,
650650
return StrtolImpl(ctx, REAL(func), nptr, endptr, base); \
651651
}
652652

653-
INTERCEPTOR_STRTO_BASE(long, strtol)
654653
INTERCEPTOR_STRTO_BASE(long long, strtoll)
655654

655+
# if SANITIZER_WINDOWS
656+
INTERCEPTOR(long, strtol, const char *nptr, char **endptr, int base) {
657+
// REAL(strtol) may be ntdll!strtol, which doesn't set errno. Instead,
658+
// call REAL(strtoll) and do the range check ourselves.
659+
COMPILER_CHECK(sizeof(long) == sizeof(u32));
660+
661+
void *ctx;
662+
ASAN_INTERCEPTOR_ENTER(ctx, strtol);
663+
AsanInitFromRtl();
664+
665+
long long result = StrtolImpl(ctx, REAL(strtoll), nptr, endptr, base);
666+
667+
if (result > INT32_MAX) {
668+
errno = errno_ERANGE;
669+
return INT32_MAX;
670+
}
671+
if (result < INT32_MIN) {
672+
errno = errno_ERANGE;
673+
return INT32_MIN;
674+
}
675+
return (long)result;
676+
}
677+
# else
678+
INTERCEPTOR_STRTO_BASE(long, strtol)
679+
# endif
680+
656681
# if SANITIZER_GLIBC
657682
INTERCEPTOR_STRTO_BASE(long, __isoc23_strtol)
658683
INTERCEPTOR_STRTO_BASE(long long, __isoc23_strtoll)

compiler-rt/lib/asan/asan_win_static_runtime_thunk.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,13 @@ INTERCEPT_LIBRARY_FUNCTION_ASAN(strpbrk);
6363
INTERCEPT_LIBRARY_FUNCTION_ASAN(strspn);
6464
INTERCEPT_LIBRARY_FUNCTION_ASAN(strstr);
6565
INTERCEPT_LIBRARY_FUNCTION_ASAN(strtok);
66-
INTERCEPT_LIBRARY_FUNCTION_ASAN(strtol);
6766
INTERCEPT_LIBRARY_FUNCTION_ASAN(wcslen);
6867
INTERCEPT_LIBRARY_FUNCTION_ASAN(wcsnlen);
6968

69+
// Note: Don't intercept strtol(l). They are supposed to set errno for out-of-
70+
// range values, but since the ASan runtime is linked against the dynamic CRT,
71+
// its errno is different from the one in the current module.
72+
7073
# if defined(_MSC_VER) && !defined(__clang__)
7174
# pragma warning(pop)
7275
# endif

compiler-rt/lib/asan/tests/asan_str_test.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,3 +638,27 @@ TEST(AddressSanitizer, StrtolOOBTest) {
638638
RunStrtolOOBTest(&CallStrtol);
639639
}
640640
#endif
641+
642+
TEST(AddressSanitizer, StrtolOverflow) {
643+
if (sizeof(long) == 4) {
644+
// Check that errno gets set correctly on 32-bit strtol overflow.
645+
long res;
646+
errno = 0;
647+
res = Ident(strtol("2147483647", NULL, 0));
648+
EXPECT_EQ(errno, 0);
649+
EXPECT_EQ(res, 2147483647);
650+
651+
res = Ident(strtol("2147483648", NULL, 0));
652+
EXPECT_EQ(errno, ERANGE);
653+
EXPECT_EQ(res, 2147483647);
654+
655+
errno = 0;
656+
res = Ident(strtol("-2147483648", NULL, 0));
657+
EXPECT_EQ(errno, 0);
658+
EXPECT_EQ(res, -2147483648);
659+
660+
res = Ident(strtol("-2147483649", NULL, 0));
661+
EXPECT_EQ(errno, ERANGE);
662+
EXPECT_EQ(res, -2147483648);
663+
}
664+
}

compiler-rt/lib/sanitizer_common/sanitizer_errno.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace __sanitizer {
2323
COMPILER_CHECK(errno_ENOMEM == ENOMEM);
2424
COMPILER_CHECK(errno_EBUSY == EBUSY);
2525
COMPILER_CHECK(errno_EINVAL == EINVAL);
26+
COMPILER_CHECK(errno_ERANGE == ERANGE);
2627

2728
// EOWNERDEAD is not present in some older platforms.
2829
#if defined(EOWNERDEAD)

compiler-rt/lib/sanitizer_common/sanitizer_errno_codes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ namespace __sanitizer {
2424
#define errno_ENOMEM 12
2525
#define errno_EBUSY 16
2626
#define errno_EINVAL 22
27+
#define errno_ERANGE 34
2728
#define errno_ENAMETOOLONG 36
2829
#define errno_ENOSYS 38
2930

compiler-rt/test/asan/TestCases/strtol_strict.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
// RUN: %env_asan_opts=strict_string_checks=true not %run %t test7 2>&1 | FileCheck %s --check-prefix=CHECK7
2424
// REQUIRES: shadow-scale-3
2525

26+
// On Windows, strtol cannot be intercepted when statically linked against the CRT.
27+
// UNSUPPORTED: win32-static-asan
28+
2629
#include <assert.h>
2730
#include <stdlib.h>
2831
#include <string.h>

0 commit comments

Comments
 (0)