Skip to content

Commit bedaea0

Browse files
ashm-devpicnixz
andauthored
gh-139269: Fix unaligned memory access in JIT code patching functions (GH-139271)
* Use memcpy for patching values instead of direct assignment Co-authored-by: Bénédikt Tran <[email protected]>
1 parent 920de7c commit bedaea0

File tree

2 files changed

+20
-11
lines changed

2 files changed

+20
-11
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix undefined behavior when using unaligned store in JIT's ``patch_*`` functions.

Python/jit.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,18 @@ set_bits(uint32_t *loc, uint8_t loc_start, uint64_t value, uint8_t value_start,
157157
uint8_t width)
158158
{
159159
assert(loc_start + width <= 32);
160+
uint32_t temp_val;
161+
// Use memcpy to safely read the value, avoiding potential alignment
162+
// issues and strict aliasing violations.
163+
memcpy(&temp_val, loc, sizeof(temp_val));
160164
// Clear the bits we're about to patch:
161-
*loc &= ~(((1ULL << width) - 1) << loc_start);
162-
assert(get_bits(*loc, loc_start, width) == 0);
165+
temp_val &= ~(((1ULL << width) - 1) << loc_start);
166+
assert(get_bits(temp_val, loc_start, width) == 0);
163167
// Patch the bits:
164-
*loc |= get_bits(value, value_start, width) << loc_start;
165-
assert(get_bits(*loc, loc_start, width) == get_bits(value, value_start, width));
168+
temp_val |= get_bits(value, value_start, width) << loc_start;
169+
assert(get_bits(temp_val, loc_start, width) == get_bits(value, value_start, width));
170+
// Safely write the modified value back to memory.
171+
memcpy(loc, &temp_val, sizeof(temp_val));
166172
}
167173

168174
// See https://developer.arm.com/documentation/ddi0602/2023-09/Base-Instructions
@@ -204,30 +210,29 @@ set_bits(uint32_t *loc, uint8_t loc_start, uint64_t value, uint8_t value_start,
204210
void
205211
patch_32(unsigned char *location, uint64_t value)
206212
{
207-
uint32_t *loc32 = (uint32_t *)location;
208213
// Check that we're not out of range of 32 unsigned bits:
209214
assert(value < (1ULL << 32));
210-
*loc32 = (uint32_t)value;
215+
uint32_t final_value = (uint32_t)value;
216+
memcpy(location, &final_value, sizeof(final_value));
211217
}
212218

213219
// 32-bit relative address.
214220
void
215221
patch_32r(unsigned char *location, uint64_t value)
216222
{
217-
uint32_t *loc32 = (uint32_t *)location;
218223
value -= (uintptr_t)location;
219224
// Check that we're not out of range of 32 signed bits:
220225
assert((int64_t)value >= -(1LL << 31));
221226
assert((int64_t)value < (1LL << 31));
222-
*loc32 = (uint32_t)value;
227+
uint32_t final_value = (uint32_t)value;
228+
memcpy(location, &final_value, sizeof(final_value));
223229
}
224230

225231
// 64-bit absolute address.
226232
void
227233
patch_64(unsigned char *location, uint64_t value)
228234
{
229-
uint64_t *loc64 = (uint64_t *)location;
230-
*loc64 = value;
235+
memcpy(location, &value, sizeof(value));
231236
}
232237

233238
// 12-bit low part of an absolute address. Pairs nicely with patch_aarch64_21r
@@ -410,7 +415,10 @@ patch_x86_64_32rx(unsigned char *location, uint64_t value)
410415
{
411416
uint8_t *loc8 = (uint8_t *)location;
412417
// Try to relax the GOT load into an immediate value:
413-
uint64_t relaxed = *(uint64_t *)(value + 4) - 4;
418+
uint64_t relaxed;
419+
memcpy(&relaxed, (void *)(value + 4), sizeof(relaxed));
420+
relaxed -= 4;
421+
414422
if ((int64_t)relaxed - (int64_t)location >= -(1LL << 31) &&
415423
(int64_t)relaxed - (int64_t)location + 1 < (1LL << 31))
416424
{

0 commit comments

Comments
 (0)