From 84bbac4f440937a3f55b1b113d2cedb63835c975 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Feb 2024 15:31:42 -0800 Subject: [PATCH 1/7] Use one mprotect call instead of two --- Python/jit.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/Python/jit.c b/Python/jit.c index dae25166b1f106..f9fab9b562386c 100644 --- a/Python/jit.c +++ b/Python/jit.c @@ -112,26 +112,6 @@ mark_executable(unsigned char *memory, size_t size) return 0; } -static int -mark_readable(unsigned char *memory, size_t size) -{ - if (size == 0) { - return 0; - } - assert(size % get_page_size() == 0); -#ifdef MS_WINDOWS - DWORD old; - int failed = !VirtualProtect(memory, size, PAGE_READONLY, &old); -#else - int failed = mprotect(memory, size, PROT_READ); -#endif - if (failed) { - jit_error("unable to protect readable memory"); - return -1; - } - return 0; -} - // JIT compiler stuff: ///////////////////////////////////////////////////////// // Warning! AArch64 requires you to get your hands dirty. These are your gloves: @@ -444,9 +424,7 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size code += group->code.body_size; data += group->data.body_size; } - if (mark_executable(memory, code_size) || - mark_readable(memory + code_size, data_size)) - { + if (mark_executable(memory, code_size + data_size)) { jit_free(memory, code_size + data_size); return -1; } From 4f21fe22ccf93d0d5c1d88b9956afb277a8057aa Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Feb 2024 21:12:37 -0800 Subject: [PATCH 2/7] Don't use separate pages for code and data --- Python/jit.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Python/jit.c b/Python/jit.c index f9fab9b562386c..38ada95995cb45 100644 --- a/Python/jit.c +++ b/Python/jit.c @@ -392,9 +392,8 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size // Round up to the nearest page (code and data need separate pages): size_t page_size = get_page_size(); assert((page_size & (page_size - 1)) == 0); - code_size += page_size - (code_size & (page_size - 1)); - data_size += page_size - (data_size & (page_size - 1)); - unsigned char *memory = jit_alloc(code_size + data_size); + size_t padding = page_size - ((code_size + data_size) & (page_size - 1)); + unsigned char *memory = jit_alloc(code_size + data_size + padding); if (memory == NULL) { return -1; } @@ -424,12 +423,12 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size code += group->code.body_size; data += group->data.body_size; } - if (mark_executable(memory, code_size + data_size)) { - jit_free(memory, code_size + data_size); + if (mark_executable(memory, code_size + data_size + padding)) { + jit_free(memory, code_size + data_size + padding); return -1; } executor->jit_code = memory; - executor->jit_size = code_size + data_size; + executor->jit_size = code_size + data_size + padding; return 0; } From 92170665fa862200dc2b2bafb4093ba2675bfc31 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Feb 2024 22:27:56 -0800 Subject: [PATCH 3/7] Fix alignment issues --- Tools/jit/_stencils.py | 5 ++--- Tools/jit/_targets.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index 78c566d9c8a7ef..e7d1e251ce26dc 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -124,7 +124,7 @@ def emit_aarch64_trampoline(self, hole: Hole) -> None: ): self.holes.append(hole.replace(offset=base + 4 * i, kind=kind)) - def remove_jump(self) -> None: + def remove_jump(self, alignment: int = 1) -> None: """Remove a zero-length continuation jump, if it exists.""" hole = max(self.holes, key=lambda hole: hole.offset) match hole: @@ -170,7 +170,7 @@ def remove_jump(self) -> None: offset -= 2 case _: return - if self.body[offset:] == jump: + if self.body[offset:] == jump and len(self.body[:offset]) % alignment == 0: self.body = self.body[:offset] self.holes.remove(hole) @@ -199,7 +199,6 @@ def process_relocations(self, *, alignment: int = 1) -> None: ): self.code.pad(alignment) self.code.emit_aarch64_trampoline(hole) - self.code.pad(alignment) self.code.holes.remove(hole) self.code.remove_jump() self.code.pad(alignment) diff --git a/Tools/jit/_targets.py b/Tools/jit/_targets.py index 417fdb56ccf7a1..66db358679239e 100644 --- a/Tools/jit/_targets.py +++ b/Tools/jit/_targets.py @@ -89,7 +89,7 @@ async def _parse(self, path: pathlib.Path) -> _stencils.StencilGroup: if group.data.body: line = f"0: {str(bytes(group.data.body)).removeprefix('b')}" group.data.disassembly.append(line) - group.process_relocations() + group.process_relocations(alignment=self.alignment) return group def _handle_section(self, section: _S, group: _stencils.StencilGroup) -> None: From 41455df2f489a079fced583b70e117e4dbeb1540 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 29 Feb 2024 22:40:10 -0800 Subject: [PATCH 4/7] Add missing alignment check --- Tools/jit/_stencils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index e7d1e251ce26dc..32ca5f3880b71e 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -124,7 +124,7 @@ def emit_aarch64_trampoline(self, hole: Hole) -> None: ): self.holes.append(hole.replace(offset=base + 4 * i, kind=kind)) - def remove_jump(self, alignment: int = 1) -> None: + def remove_jump(self, *, alignment: int = 1) -> None: """Remove a zero-length continuation jump, if it exists.""" hole = max(self.holes, key=lambda hole: hole.offset) match hole: @@ -200,7 +200,7 @@ def process_relocations(self, *, alignment: int = 1) -> None: self.code.pad(alignment) self.code.emit_aarch64_trampoline(hole) self.code.holes.remove(hole) - self.code.remove_jump() + self.code.remove_jump(alignment=alignment) self.code.pad(alignment) self.data.pad(8) for stencil in [self.code, self.data]: From 36c6c78af6149b7bfe589782a22bba7dbe89781b Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 14 Mar 2024 09:28:08 -0700 Subject: [PATCH 5/7] Clean up alignement calculation --- Tools/jit/_stencils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index 32ca5f3880b71e..05c4ce8249f687 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -170,7 +170,7 @@ def remove_jump(self, *, alignment: int = 1) -> None: offset -= 2 case _: return - if self.body[offset:] == jump and len(self.body[:offset]) % alignment == 0: + if self.body[offset:] == jump and offset % alignment == 0: self.body = self.body[:offset] self.holes.remove(hole) From 26c31d995d1317cfd1dbf423a3339b84196bc336 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 15 Mar 2024 10:58:33 -0700 Subject: [PATCH 6/7] Be extra paranoid --- Python/jit.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Python/jit.c b/Python/jit.c index 38ada95995cb45..b75f021df5e925 100644 --- a/Python/jit.c +++ b/Python/jit.c @@ -389,6 +389,8 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size code_size += group->code.body_size; data_size += group->data.body_size; } + code_size += stencil_groups[_FATAL_ERROR].code.body_size; + data_size += stencil_groups[_FATAL_ERROR].data.body_size; // Round up to the nearest page (code and data need separate pages): size_t page_size = get_page_size(); assert((page_size & (page_size - 1)) == 0); @@ -423,6 +425,20 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size code += group->code.body_size; data += group->data.body_size; } + // Protect against accidental buffer overrun into data: + const StencilGroup *group = &stencil_groups[_FATAL_ERROR]; + uint64_t patches[] = GET_PATCHES(); + patches[HoleValue_CODE] = (uint64_t)code; + patches[HoleValue_CONTINUE] = (uint64_t)code; + patches[HoleValue_DATA] = (uint64_t)data; + patches[HoleValue_EXECUTOR] = (uint64_t)executor; + patches[HoleValue_TOP] = (uint64_t)code; + patches[HoleValue_ZERO] = 0; + emit(group, patches); + code += group->code.body_size; + data += group->data.body_size; + assert(code == memory + code_size); + assert(data == memory + code_size + data_size); if (mark_executable(memory, code_size + data_size + padding)) { jit_free(memory, code_size + data_size + padding); return -1; From e6d8e6d45cc1d064ace224a92ae85b141133177e Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Fri, 15 Mar 2024 12:47:40 -0700 Subject: [PATCH 7/7] total_size --- Python/jit.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Python/jit.c b/Python/jit.c index b75f021df5e925..f67d641fe129e1 100644 --- a/Python/jit.c +++ b/Python/jit.c @@ -391,11 +391,12 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size } code_size += stencil_groups[_FATAL_ERROR].code.body_size; data_size += stencil_groups[_FATAL_ERROR].data.body_size; - // Round up to the nearest page (code and data need separate pages): + // Round up to the nearest page: size_t page_size = get_page_size(); assert((page_size & (page_size - 1)) == 0); size_t padding = page_size - ((code_size + data_size) & (page_size - 1)); - unsigned char *memory = jit_alloc(code_size + data_size + padding); + size_t total_size = code_size + data_size + padding; + unsigned char *memory = jit_alloc(total_size); if (memory == NULL) { return -1; } @@ -439,12 +440,12 @@ _PyJIT_Compile(_PyExecutorObject *executor, const _PyUOpInstruction *trace, size data += group->data.body_size; assert(code == memory + code_size); assert(data == memory + code_size + data_size); - if (mark_executable(memory, code_size + data_size + padding)) { - jit_free(memory, code_size + data_size + padding); + if (mark_executable(memory, total_size)) { + jit_free(memory, total_size); return -1; } executor->jit_code = memory; - executor->jit_size = code_size + data_size + padding; + executor->jit_size = total_size; return 0; }