Skip to content

Commit 914ce6f

Browse files
Avoid all compiler optimization on embedded apphost hash (#110554)
* Avoid all compiler optimization on embedded apphost hash We assume that there is a single copy of the apphost hash in the apphost binary. And that it hasn't been modified by the compiler. However, the compiler can optimize the hash multiple ways, including re-ordering elements of the hash or duplicating the contents of the hash. This can currently happen under certain compiler versions and optimization flags. Try and avoid that by marking the hash as a volatile string and implementing comparisons that respects that. --------- Co-authored-by: Aaron R Robinson <[email protected]>
1 parent 50e6f48 commit 914ce6f

File tree

1 file changed

+20
-7
lines changed

1 file changed

+20
-7
lines changed

src/native/corehost/corehost.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,27 @@
4040
#define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
4141
#define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
4242

43+
// This avoids compiler optimization which cause EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8
44+
// to be placed adjacent causing them to match EMBED_HASH_FULL_UTF8 when searched for replacing.
45+
// See https://github.com/dotnet/runtime/issues/109611 for more details.
46+
static bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
47+
{
48+
for (size_t i = 0; i < length; i++)
49+
{
50+
if (*a++ != *b++)
51+
return false;
52+
}
53+
return true;
54+
}
55+
4356
bool is_exe_enabled_for_execution(pal::string_t* app_dll)
4457
{
4558
constexpr int EMBED_SZ = sizeof(EMBED_HASH_FULL_UTF8) / sizeof(EMBED_HASH_FULL_UTF8[0]);
4659
constexpr int EMBED_MAX = (EMBED_SZ > 1025 ? EMBED_SZ : 1025); // 1024 DLL name length, 1 NUL
4760

4861
// Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
49-
// Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
50-
// where length is determined at compile time (=64) instead of the actual length of the string at runtime.
62+
// Must not be 'const' because strlen below could be determined at compile time (=64) instead of the actual
63+
// length of the string at runtime.
5164
static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
5265

5366
static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
@@ -59,10 +72,10 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
5972
return false;
6073
}
6174

62-
std::string binding(&embed[0]);
75+
size_t binding_len = strlen(&embed[0]);
6376

6477
// Check if the path exceeds the max allowed size
65-
if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
78+
if (binding_len > EMBED_MAX - 1) // -1 for null terminator
6679
{
6780
trace::error(_X("The managed DLL bound to this executable is longer than the max allowed length (%d)"), EMBED_MAX - 1);
6881
return false;
@@ -73,9 +86,9 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
7386
// So use two parts of the string that will be unaffected by the edit.
7487
size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1;
7588
size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1;
76-
if (binding.size() >= (hi_len + lo_len)
77-
&& binding.compare(0, hi_len, &hi_part[0]) == 0
78-
&& binding.compare(hi_len, lo_len, &lo_part[0]) == 0)
89+
if (binding_len >= (hi_len + lo_len)
90+
&& compare_memory_nooptimization(&embed[0], hi_part, hi_len)
91+
&& compare_memory_nooptimization(&embed[hi_len], lo_part, lo_len))
7992
{
8093
trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str());
8194
return false;

0 commit comments

Comments
 (0)