pyrox.dev
1From 4e333377f97ab8f0f47ba7606844c34cb61d1db0 Mon Sep 17 00:00:00 2001
2From: Omair Majid <omajid@redhat.com>
3Date: Mon, 9 Dec 2024 17:44:10 -0500
4Subject: [PATCH 1/4] Avoid all compiler optimization on embedded apphost hash
5
6We assume that there is a single copy of the apphost hash in the apphost
7binary. And that it hasn't been modified by the compiler. However, the
8compiler can optimize the hash multiple ways, including re-ordering
9elements of the hash or duplicating the contents of the hash. This can
10currently happen under certain compiler versions and optimization flags.
11
12Try and avoid that by marking the hash as a volatile string and
13implementing comparisons/copying/initialization that respects that.
14
15Fixes: #109611
16---
17 src/runtime/src/native/corehost/corehost.cpp | 31 ++++++++++++++++++++++++++-----
18 1 file changed, 26 insertions(+), 5 deletions(-)
19
20diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
21index 6de7acfbd08576..6d40a337d574a2 100644
22--- a/src/runtime/src/native/corehost/corehost.cpp
23+++ b/src/runtime/src/native/corehost/corehost.cpp
24@@ -40,6 +40,24 @@
25 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
26 #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
27
28+void to_non_volatile(volatile const char* cstr, char* output, size_t length)
29+{
30+ for (size_t i = 0; i < length; i++)
31+ {
32+ output[i] = cstr[i];
33+ }
34+}
35+
36+bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
37+{
38+ for (size_t i = 0; i < length; i++)
39+ {
40+ if (*a++ != *b++)
41+ return false;
42+ }
43+ return true;
44+}
45+
46 bool is_exe_enabled_for_execution(pal::string_t* app_dll)
47 {
48 constexpr int EMBED_SZ = sizeof(EMBED_HASH_FULL_UTF8) / sizeof(EMBED_HASH_FULL_UTF8[0]);
49@@ -48,18 +66,21 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
50 // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
51 // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
52 // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
53- static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
54+ volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
55
56 static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
57 static const char lo_part[] = EMBED_HASH_LO_PART_UTF8;
58
59- if (!pal::clr_palstring(embed, app_dll))
60+ char working_copy_embed[EMBED_MAX];
61+ to_non_volatile(embed, working_copy_embed, EMBED_MAX);
62+
63+ if (!pal::clr_palstring(&working_copy_embed[0], app_dll))
64 {
65 trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image."));
66 return false;
67 }
68
69- std::string binding(&embed[0]);
70+ std::string binding(&working_copy_embed[0]);
71
72 // Check if the path exceeds the max allowed size
73 if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
74@@ -74,8 +95,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
75 size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1;
76 size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1;
77 if (binding.size() >= (hi_len + lo_len)
78- && binding.compare(0, hi_len, &hi_part[0]) == 0
79- && binding.compare(hi_len, lo_len, &lo_part[0]) == 0)
80+ && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len)
81+ && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len))
82 {
83 trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str());
84 return false;
85
86From 2c67debff3f84519b7b5cba49232aaa2396a9f3e Mon Sep 17 00:00:00 2001
87From: Aaron R Robinson <arobins@microsoft.com>
88Date: Wed, 26 Mar 2025 20:40:51 -0700
89Subject: [PATCH 2/4] Apply feedback
90
91---
92 src/runtime/src/native/corehost/corehost.cpp | 20 ++++++--------------
93 1 file changed, 6 insertions(+), 14 deletions(-)
94
95diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
96index 6d40a337d574a2..9d2648c0ba84fa 100644
97--- a/src/runtime/src/native/corehost/corehost.cpp
98+++ b/src/runtime/src/native/corehost/corehost.cpp
99@@ -40,14 +40,9 @@
100 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
101 #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
102
103-void to_non_volatile(volatile const char* cstr, char* output, size_t length)
104-{
105- for (size_t i = 0; i < length; i++)
106- {
107- output[i] = cstr[i];
108- }
109-}
110-
111+// This is a workaround for a compiler workaround that
112+// causes issues with inserting multiple static strings.
113+// See https://github.com/dotnet/runtime/issues/109611 for more details.
114 bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
115 {
116 for (size_t i = 0; i < length; i++)
117@@ -66,21 +61,18 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
118 // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
119 // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
120 // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
121- volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
122+ static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
123
124 static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
125 static const char lo_part[] = EMBED_HASH_LO_PART_UTF8;
126
127- char working_copy_embed[EMBED_MAX];
128- to_non_volatile(embed, working_copy_embed, EMBED_MAX);
129-
130- if (!pal::clr_palstring(&working_copy_embed[0], app_dll))
131+ if (!pal::clr_palstring(embed, app_dll))
132 {
133 trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image."));
134 return false;
135 }
136
137- std::string binding(&working_copy_embed[0]);
138+ std::string binding(&embed[0]);
139
140 // Check if the path exceeds the max allowed size
141 if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
142
143From 854143d39e7725d82547032f1ab47ea5da062b9f Mon Sep 17 00:00:00 2001
144From: Aaron R Robinson <arobins@microsoft.com>
145Date: Thu, 27 Mar 2025 19:04:09 -0700
146Subject: [PATCH 3/4] Feedback
147
148---
149 src/runtime/src/native/corehost/corehost.cpp | 16 ++++++++--------
150 1 file changed, 8 insertions(+), 8 deletions(-)
151
152diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
153index 9d2648c0ba84fa..36902ccfa56c04 100644
154--- a/src/runtime/src/native/corehost/corehost.cpp
155+++ b/src/runtime/src/native/corehost/corehost.cpp
156@@ -40,10 +40,10 @@
157 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
158 #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
159
160-// This is a workaround for a compiler workaround that
161-// causes issues with inserting multiple static strings.
162+// This avoids compiler optimization which cause EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8
163+// to be placed adjacent causing them to match EMBED_HASH_FULL_UTF8 when searched for replacing.
164 // See https://github.com/dotnet/runtime/issues/109611 for more details.
165-bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
166+static bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
167 {
168 for (size_t i = 0; i < length; i++)
169 {
170@@ -72,10 +72,10 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
171 return false;
172 }
173
174- std::string binding(&embed[0]);
175+ size_t binding_len = strlen(&embed[0]);
176
177 // Check if the path exceeds the max allowed size
178- if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
179+ if (binding_len > EMBED_MAX - 1) // -1 for null terminator
180 {
181 trace::error(_X("The managed DLL bound to this executable is longer than the max allowed length (%d)"), EMBED_MAX - 1);
182 return false;
183@@ -86,9 +86,9 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
184 // So use two parts of the string that will be unaffected by the edit.
185 size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1;
186 size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1;
187- if (binding.size() >= (hi_len + lo_len)
188- && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len)
189- && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len))
190+ if (binding_len >= (hi_len + lo_len)
191+ && compare_memory_nooptimization(&embed[0], hi_part, hi_len)
192+ && compare_memory_nooptimization(&embed[hi_len], lo_part, lo_len))
193 {
194 trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str());
195 return false;
196
197From 842d62e499ce6511abf948cf5da8023cc6be8212 Mon Sep 17 00:00:00 2001
198From: Aaron R Robinson <arobins@microsoft.com>
199Date: Fri, 28 Mar 2025 15:44:47 -0700
200Subject: [PATCH 4/4] Feedback
201
202---
203 src/runtime/src/native/corehost/corehost.cpp | 4 ++--
204 1 file changed, 2 insertions(+), 2 deletions(-)
205
206diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
207index 36902ccfa56c04..54eb128cb486bb 100644
208--- a/src/runtime/src/native/corehost/corehost.cpp
209+++ b/src/runtime/src/native/corehost/corehost.cpp
210@@ -59,8 +59,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
211 constexpr int EMBED_MAX = (EMBED_SZ > 1025 ? EMBED_SZ : 1025); // 1024 DLL name length, 1 NUL
212
213 // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
214- // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
215- // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
216+ // Must not be 'const' because strlen below could be determined at compile time (=64) instead of the actual
217+ // length of the string at runtime.
218 static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string
219
220 static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;