gh-128605: Add branch protections for x86_64 in asm_trampoline.S #128606

stratakis · 2025-01-08T04:51:26Z

Issue: asm_trampoline.S misses branch protection flags for x86_64 and aarch64 #128605

stratakis · 2025-01-08T04:53:46Z

stratakis · 2025-01-08T04:55:37Z

In the current implementation the cf-protection test for x86_64 passes but not the branch-protection test for aarch64.

fweimer-rh · 2025-01-08T06:58:14Z

You should add PAC support at the same time, using hint 25 (paciasp, replaces bti c) and hint 29 (autiasp). These instructions are in the NOP space, but some CPUs have trouble executing them. This applies to bit c as well. So it's best to make this conditional on compiler flags, by checking the __ARM_FEATURE_BTI_DEFAULT preprocessor macro. Then you can change the flag at the end of the note from 1 to 3, and this should make annocheck happy.

stratakis · 2025-01-09 8000 T14:28:41Z

Rebased. The conditionals are a bit of a mess right now so I'll do some macros later on.

However annobin still reports:

Hardened: libpython3.12.so.1.0: FAIL: dynamic-tags test because the BTI_PLT flag is missing from the dynamic tags
Hardened: libpython3.12.so.1.0: FAIL: property-note test because properly formatted .note.gnu.property not found (it is needed for branch protection support)

Btw the arm blog proposes a different styled note than what the annobin docs mention: https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/p2-enabling-pac-and-bti-on-aarch64

Also regarding x86_64. Shouldn't the note be conditionalized in the case CET is active?

stratakis · 2025-01-12T18:16:19Z

cc'ing also @pablogsal as the original author of the file.

bedevere-bot · 2025-01-13T00:13:21Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 549b894 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

pablogsal

This will break the DWARF mode as the debug information we are adding matches exactly with the current assembly instructions and positions:

cpython/Python/perf_jit_trampoline.c

Lines 452 to 489 in d0ecbdd

    
               /* Emit DWARF EH CIE. */ 
        
               DWRF_SECTION(CIE, DWRF_U32(0); /* Offset to CIE itself. */ 
        
                            DWRF_U8(DWRF_CIE_VERSION); 
        
                            DWRF_STR("zR"); /* Augmentation. */ 
        
                            DWRF_UV(1); /* Code alignment factor. */ 
        
                            DWRF_SV(-(int64_t)sizeof(uintptr_t)); /* Data alignment factor. */ 
        
                            DWRF_U8(DWRF_REG_RA); /* Return address register. */ 
        
                            DWRF_UV(1); 
        
                            DWRF_U8(DWRF_EH_PE_pcrel | DWRF_EH_PE_sdata4); /* Augmentation data. */ 
        
                            DWRF_U8(DWRF_CFA_def_cfa); DWRF_UV(DWRF_REG_SP); DWRF_UV(sizeof(uintptr_t)); 
        
                            DWRF_U8(DWRF_CFA_offset|DWRF_REG_RA); DWRF_UV(1); 
        
                            DWRF_ALIGNNOP(sizeof(uintptr_t)); 
        
               ) 
        
               ctx->eh_frame_p = p; 
        
               /* Emit DWARF EH FDE. */ 
        
               DWRF_SECTION(FDE, DWRF_U32((uint32_t)(p - framep)); /* Offset to CIE. */ 
        
                            DWRF_U32(-0x30); /* Machine code offset relative to .text. */ 
        
                            DWRF_U32(ctx->code_size); /* Machine code length. */ 
        
                            DWRF_U8(0); /* Augmentation data. */ 
        
               /* Registers saved in CFRAME. */ 
        
           #ifdef __x86_64__ 
        
                            DWRF_U8(DWRF_CFA_advance_loc | 4); 
        
                            DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16); 
        
                            DWRF_U8(DWRF_CFA_advance_loc | 6); 
        
                            DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(8); 
        
               /* Extra registers saved for JIT-compiled code. */ 
        
           #elif defined(__aarch64__) && defined(__AARCH64EL__) && !defined(__ILP32__) 
        
                            DWRF_U8(DWRF_CFA_advance_loc | 1); 
        
                            DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16); 
        
                            DWRF_U8(DWRF_CFA_offset | 29); DWRF_UV(2); 
        
                            DWRF_U8(DWRF_CFA_offset | 30); DWRF_UV(1); 
        
                            DWRF_U8(DWRF_CFA_advance_loc | 3); 
        
                            DWRF_U8(DWRF_CFA_offset | -(64 - 29)); 
        
                            DWRF_U8(DWRF_CFA_offset | -(64 - 30)); 
        
                            DWRF_U8(DWRF_CFA_def_cfa_offset); 
        
                            DWRF_UV(0);

I am also not sure why this is needed other than make the annocheck tool happy so I am not sure we want to add this extra complexity.

bedevere-app · 2025-01-13T00:15:46Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

stratakis · 2025-01-13T14:17:27Z

I am also not sure why this is needed other than make the annocheck tool happy so I am not sure we want to add this extra complexity.

The justification for this change is provided at the linked issue.

pablogsal · 2025-01-13T15:17:48Z

I am also not sure why this is needed other than make the annocheck tool happy so I am not sure we want to add this extra complexity.

The justification for this change is provided at the linked issue.

I don't think that justification is enough. The only thing the issue mentioned is that we are not using a feature and that the annocheck is unhappy. Could you ellaborate what are the consequences of not adding these instructions?

In any case, we need to check if the DWARF needs to be updated to account for the new instructions and if GDB can still unwind across them.

stratakis · 2025-01-14T00:51:13Z

I don't think that justification is enough. The only thing the issue mentioned is that we are not using a feature and that the annocheck is unhappy. Could you ellaborate what are the consequences of not adding these instructions?

Of course. Both ubuntu and Fedora (and by extension RHEL) and I assume other distros, enable by default the -fcf-protection flag for x86_64 and -mbranch-protection=standard for aarch64. When these flags are applied, the compiler places those instructions at the appropriate places when the CPU supports them and a NOP when it doesn't.

Both are implemented to mitigate return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks.

Minimum supported CPU models are Intel's Tiger lake, AMD's Zen 3, Arm 8.3 (PAC) and Arm 8.5 (BTI).

With pure C code everything is fine by just using the compiler 8000 flag(s), however when an assembly file is added to the mix the instructions need to be added manually.

This feature is an all-or-nothing for the hardware functionality to work. All object files are checked for the note and if it's missing on any one, the linker does not place the note to the final executable and the hardening feature is disabled in the hardware. That essentially means that for Python 3.12+ these flags have no effect anymore.

pablogsal · 2025-01-14T13:01:57Z

Ok makes sense. Thanks a lot for the extra context.

We need to complement the DWARF information to ensure that the Perf JIT integration still works and we need to ensure gdb can still unwind through the trampolines when this information is active.

brandtbucher · 2025-01-14T19:03:56Z

This feature is an all-or-nothing for the hardware functionality to work. All object files are checked for the note and if it's missing on any one, the linker does not place the note to the final executable and the hardening feature is disabled in the hardware. That essentially means that for Python 3.12+ these flags have no effect anymore.

Are JIT compilers expected to implement this as well? Or no, since they aren't statically present in the executable?

pablogsal · 2025-01-14T23:29:26Z

This feature is an all-or-nothing for the hardware functionality to work. All object files are checked for the note and if it's missing on any one, the linker does not place the note to the final executable and the hardening feature is disabled in the hardware. That essentially means that for Python 3.12+ these flags have no effect anymore.

Are JIT compilers expected to implement this as well? Or no, since they aren't statically present in the executable?

Technically this code is also not present in the executable: we copy it from memory but the region itself where we copy it from is never executed. So if the answer is "no" then we shouldn't need this either

stratakis · 2025-01-15T01:04:32Z

Ok makes sense. Thanks a lot for the extra context.

We need to complement the DWARF information to ensure that the Perf JIT integration still works and we need to ensure gdb can still unwind through the trampolines when this information is active.

Providing also a more practical reproducer.

On Python 3.11 on x86_64:

$ ./configure && make -j10 CFLAGS=-fcf-protection
$ readelf -n python

Properties: x86 feature: IBT, SHSTK

Both of those flags indicate that CET protection is enabled. In Python 3.12+ they are not present.

Of course when building with --enable-shared libpython needs to be inspected instead.

I'll familiarize myself with the debug information and try to figure it out.

stratakis · 2025-01-15T01:43:01Z

This feature is an all-or-nothing for the hardware functionality to work. All object files are checked for the note and if it's missing on any one, the linker does not place the note to the final executable and the hardening feature is disabled in the hardware. That essentially means that for Python 3.12+ these flags have no effect anymore.

Are JIT compilers expected to implement this as well? Or no, since they aren't statically present in the executable?

The answer here would be possibly yes, assembly instructions emitted by JITs should implement that, however I am not familiar myself with JITs in general or the way CPython implements it to provide a definite answer. As JIT is still being developed I'd treat this as an afterthought and revisit it in the future.

fweimer-rh · 2025-01-15T07:37:49Z

Ok makes sense. Thanks a lot for the extra context.

We need to complement the DWARF information to ensure that the Perf JIT integration still works and we need to ensure gdb can still unwind through the trampolines when this information is active.

@pablogsal As the DWARF needs to be updated anyway, it may make sense to add the missing frame pointer to the x86-64 assembler (if Python's policy is to have frame pointers). Or is the goal to elide the calling frame from profiling backtraces? If the latter, there should really be a comment to this effect.

fweimer-rh · 2025-01-15T07:43:35Z

Are JIT compilers expected to implement this as well? Or no, since they aren't statically present in the executable?

@brandtbucher For JIT compilers to keep working unchanged, they need to be marked as incompatible for now. On x86-64, SHSTK is generally automatically compatible with JIT compilation unless stack unwinding code is generated. However, x86-64 IBT requires marker instructions at the target of indirect branches. On AArch64, BTI is similar (marker instructions), but PAC is not process-switched on Linux: it's determined at boot whether it is turned on or not. The presence of the extra instructions merely ensures that the JIT code does not contain any easy-to-use JOP/ROP gadgets.

Whether any of this matters is of course debatable, assuming that Python does not write-protect any of its bytecode in the process image.

pablogsal · 2025-01-15T08:28:20Z

Ok makes sense. Thanks a lot for the extra context.

We need to complement the DWARF information to ensure that the Perf JIT integration still works and we need to ensure gdb can still unwind through the trampolines when this information is active.

@pablogsal As the DWARF needs to be updated anyway, it may make sense to add the missing frame pointer to the x86-64 assembler (if Python's policy is to have frame pointers). Or is the goal to elide the calling frame from profiling backtraces? If the latter, there should really be a comment to this effect.

One of the problems is that if you add the frame pointer to the x86-64 assembler gdb cannot unwind through this when Python is compiled without frame pointers. The current version allows unwinding with and without frame pointers.

brandtbucher · 2025-01-16T00:30:51Z

@brandtbucher For JIT compilers to keep working unchanged, they need to be marked as incompatible for now. On x86-64, SHSTK is generally automatically compatible with JIT compilation unless stack unwinding code is generated. However, x86-64 IBT requires marker instructions at the target of indirect branches. On AArch64, BTI is similar (marker instructions), but PAC is not process-switched on Linux: it's determined at boot whether it is turned on or not. The presence of the extra instructions merely ensures that the JIT code does not contain any easy-to-use JOP/ROP gadgets.

Whether any of this matters is of course debatable, assuming that Python does not write-protect any of its bytecode in the process image.

I'm afraid this is a bit too jargon-heavy for me to follow without doing a bunch of self-guided research on the topic.

What does it mean for the JIT code to be "marked incompatible"? I take it that if the process/kernel has these hardware features enabled, JIT code that doesn't use it will probably crash? What's the typical performance overhead of enabling this, and who exactly is using it (Ubuntu and Fedora were mentioned above... is it all users of these distros who would be unable to use our JIT)?

miss-islington-app · 2025-06-03T07:09:46Z

Thanks @stratakis for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13, 3.14.
🐍🍒⛏🤖

bedevere-bot · 2025-06-03T07:28:59Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x (tier-1) has failed when building commit 485b499.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/345/builds/11525) and take a look at the build logs.
Check if the failure is related to this commit (485b499) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/345/builds/11525

Failed tests:

test.test_concurrent_futures.test_process_pool
test_cmd
test_difflib
test_buffer
test_zipapp
test_peg_generator
test_ucn
test_optparse
test_codecencodings_kr
test_statistics

Failed subtests:

test_html_diff - test.test_difflib.TestSFpatches.test_html_diff
test_ternary_operator - test.test_peg_generator.test_c_parser.TestCParser.test_ternary_operator
test_memoryview_cast_invalid - test.test_buffer.TestBufferProtocol.test_memoryview_cast_invalid
test_map_exception - test.test_concurrent_futures.test_process_pool.ProcessPoolForkserverProcessPoolExecutorTest.test_map_exception

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_peg_generator/test_c_parser.py", line 413, in test_ternary_operator
    self.run_test(grammar_source, test_source)
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_peg_generator/test_c_parser.py", line 136, in run_test
    assert_python_ok(
    ~~~~~~~~~~~~~~~~^
        "-c",
        ^^^^^
        TEST_TEMPLATE.format(extension_path=self.tmp_path, test_source=test_source),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/script_helper.py", line 182, in assert_python_ok
    return _assert_python(True, *args, **env_vars)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/script_helper.py", line 167, in _assert_python
    res.fail(cmd_line)
    ~~~~~~~~^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/script_helper.py", line 80, in fail
    raise AssertionError(f"Process return code is {exitcode}\n"
    ...<10 lines>...
                         f"---")
AssertionError: Process return code is -6 (SIGABRT)
command line: ['/root/buildarea/3.x.angelico-debian-amd64/build/python', '-X', 'faulthandler', '-I', '-c', '\ntmp_dir = \'/root/buildarea/3.x.angelico-debian-amd64/build/build/test_python_1500507æ/tmpf33ta7nm\'\n\nimport ast\nimport traceback\nimport sys\nimport unittest\n\nfrom test import test_tools\nwith test_tools.imports_under_tool("peg_generator"):\n    from pegen.ast_dump import ast_dump\n\nsys.path.insert(0, tmp_dir)\nimport parse\n\nclass Tests(unittest.TestCase):\n\n    def check_input_strings_for_grammar(\n        self,\n        valid_cases = (),\n        invalid_cases = (),\n    ):\n        if valid_cases:\n            for case in valid_cases:\n                parse.parse_string(case, mode=0)\n\n        if invalid_cases:\n            for case in invalid_cases:\n                with self.assertRaises(SyntaxError):\n                    parse.parse_string(case, mode=0)\n\n    def verify_ast_generation(self, stmt):\n        expected_ast = ast.parse(stmt)\n        actual_ast = parse.parse_string(stmt, mode=1)\n        self.assertEqual(ast_dump(expected_ast), ast_dump(actual_ast))\n\n    def test_parse(self):\n        \n        stmt = "[i for i in a if b]"\n        self.verify_ast_generation(stmt)\n\n\nunittest.main()\n']


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 198, in _run_module_as_main
    return _run_code(code, main_globals, None,
                     "__main__", mod_spec)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/runpy.py", line 88, in _run_code
    exec(code, run_globals)
    ~~~~^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/libregrtest/worker.py", line 6, in <module>
    from test.support import os_helper, Py_DEBUG
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/support/__init__.py", line 6, in <module>
    import annotationlib
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/annotationlib.py", line 22, in <module>
    class Format(enum.IntEnum):
    ...<3 lines>...
        STRING = 4
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/annotationlib.py", line 22, in Format
    class Format(enum.IntEnum):
    
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/enum.py", line 350, in __setitem__
    if self._cls_name is not None and _is_private(self._cls_name, key):
                                      ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
TypeError: 'str' object is not callable


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_concurrent_futures/util.py", line 54, in setUp
    self.manager = self.get_context().Manager()
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/multiprocessing/context.py", line 57, in Manager
    m.start()
    ~~~~~~~^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/multiprocessing/managers.py", line 570, in start
    self._address = reader.recv()
                    ~~~~~~~~~~~^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/multiprocessing/connection.py", line 256, in recv
    buf = self._recv_bytes()
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/multiprocessing/connection.py", line 447, in _recv_bytes
    buf = self._recv(4)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/multiprocessing/connection.py", line 416, in _recv
    raise EOFError
EOFError


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_difflib.py", line 236, in test_html_diff
    k.make_table(f3.splitlines(True),t3.splitlines(True)),
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 2017, in make_table
    fromlist,tolist,flaglist = self._collect_lines(diffs)
                               ~~~~~~~~~~~~~~~~~~~^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 1885, in _collect_lines
    for fromdata,todata,flag in diffs:
                                ^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 1852, in _line_wrapper
    for fromdata,todata,flag in diffs:
                                ^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 1568, in _mdiff
    yield from line_pair_iterator
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 1552, in _line_pair_iterator
    from_line, to_line, found_diff = next(line_iterator)
                                     ~~~~^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 1466, in _line_iterator
    lines.append(next(diff_lines_iterator, 'X'))
                 ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 872, in compare
    yield from g
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 960, in _fancy_replace
    for tag, ai1, ai2, bj1, bj2 in cruncher.get_opcodes():
                                   ~~~~~~~~~~~~~~~~~~~~^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 525, in get_opcodes
    for ai, bj, size in self.get_matching_blocks():
                        ~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/difflib.py", line 489, in get_matching_blocks
    self.matching_blocks = list(map(Match._make, non_adjacent))
                           ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/collections/__init__.py", line 456, in _make
    if _len(result) != num_fields:
       ~~~~^^^^^^^^
TypeError: object of type 'type' has no len()


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 2604, in test_memoryview_cast_invalid
    for dfmt, _, _ in iter_format(1):
                      ~~~~~~~~~~~^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 233, in iter_format
    for t in iter_mode(nitems, testobj):
             ~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 228, in iter_mode
    yield randitems(n, obj, mode, char)
          ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 219, in randitems
    items = gen_items(n, fmt, obj)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 194, in gen_items
    lst[i] = gen_item(fmt, obj)
             ~~~~~~~~^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 185, in gen_item
    x.append(randrange_fmt(mode, c, obj))
             ~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_buffer.py", line 167, in randrange_fmt
    x = randrange(*fmtdict[mode][char])
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/random.py", line 321, in randrange
    return istart + self._randbelow(width)
                    ~~~~~~~~~~~~~~~^^^^^^^
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/random.py", line 249, in _randbelow_with_getrandbits
    r = self.getrandbits(k)  # 0 <= r < 2**k
TypeError: 'Random' object cannot be interpreted as an integer

miss-islington-app · 2025-06-03T10:25:42Z

Thanks @stratakis for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington-app · 2025-06-03T10:25:42Z

Thanks @stratakis for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington-app · 2025-06-03T10:25:42Z

Thanks @stratakis for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2025-06-03T10:25:45Z

Sorry, @stratakis and @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 485b499610eefe362faf171f258b3a3588378ff1 3.12

miss-islington-app · 2025-06-03T10:25:49Z

Sorry, @stratakis and @vstinner, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 485b499610eefe362faf171f258b3a3588378ff1 3.14

miss-islington-app · 2025-06-03T10:25:52Z

Sorry, @stratakis and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 485b499610eefe362faf171f258b3a3588378ff1 3.13

vstinner · 2025-06-03T12:07:35Z

@stratakis: Do you want to try to backport manually this change to the 3.14 branch?

python#128606) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html

…poline.S (python#128606) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html

bedevere-app · 2025-06-03T12:43:36Z

GH-135077 is a backport of this pull request to the 3.14 branch.

….S (#128606) (#135077) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html

…poline.S (pythonGH-128606) (pythonGH-135077) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html (cherry picked from commit 899cca6) Co-authored-by: stratakis <cstratak@redhat.com>

vstinner · 2025-06-03T13:33:09Z

Merged, thank you.

….S (GH-128606) (GH-135077) (#135083) [3.14] gh-128605: Add branch protections for x86_64 in asm_trampoline.S (GH-128606) (GH-135077) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html (cherry picked from commit 899cca6) Co-authored-by: stratakis <cstratak@redhat.com>

…poline.S (python#128606) Apply Intel Control-flow Technology for x86-64 on asm_trampoline.S. Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks. Manual application is required for the assembly files. See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html

bedevere-app · 2025-06-03T14:43:48Z

GH-135094 is a backport of this pull request to the 3.12 branch.

Required for mitigation against return-oriented programming (ROP) and Call or Jump Oriented Programming (COP/JOP) attacks Proposed upstream: python#128606 See also: https://sourceware.org/annobin/annobin.html/Test-cf-protection.html

bedevere-app bot mentioned this pull request Jan 8, 2025

asm_trampoline.S misses branch protection flags for x86_64 and aarch64 #128605

Open

bedevere-app bot added the awaiting review label Jan 8, 2025

stratakis force-pushed the branch_protection branch from d97e78d to 6d86e0f Compare January 9, 2025 14:25

stratakis force-pushed the branch_protection branch from 6d86e0f to 549b894 Compare January 9, 2025 14:30

pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 13, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 13, 2025

pablogsal previously requested changes Jan 13, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting review labels Jan 13, 2025

stratakis force-pushed the branch_protection branch 2 times, most recently from a7408db to 6d2d4f7 Compare January 14, 2025 02:39

github-project-automation bot moved this to Done in lavitaconnect@MOSTAFAAMMER Jun 3, 2025

bedevere-app bot removed the awaiting merge label Jun 3, 2025

miss-islington-app bot assigned vstinner Jun 3, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 3, 2025

bedevere-app bot removed the needs backport to 3.12 only security fixes label Jun 3, 2025

	/* Emit DWARF EH CIE. */
	DWRF_SECTION(CIE, DWRF_U32(0); /* Offset to CIE itself. */
	DWRF_U8(DWRF_CIE_VERSION);
	DWRF_STR("zR"); /* Augmentation. */
	DWRF_UV(1); /* Code alignment factor. */
	DWRF_SV(-(int64_t)sizeof(uintptr_t)); /* Data alignment factor. */
	DWRF_U8(DWRF_REG_RA); /* Return address register. */
	DWRF_UV(1);
	DWRF_U8(DWRF_EH_PE_pcrel \| DWRF_EH_PE_sdata4); /* Augmentation data. */
	DWRF_U8(DWRF_CFA_def_cfa); DWRF_UV(DWRF_REG_SP); DWRF_UV(sizeof(uintptr_t));
	DWRF_U8(DWRF_CFA_offset\|DWRF_REG_RA); DWRF_UV(1);
	DWRF_ALIGNNOP(sizeof(uintptr_t));
	)

	ctx->eh_frame_p = p;

	/* Emit DWARF EH FDE. */
	DWRF_SECTION(FDE, DWRF_U32((uint32_t)(p - framep)); /* Offset to CIE. */
	DWRF_U32(-0x30); /* Machine code offset relative to .text. */
	DWRF_U32(ctx->code_size); /* Machine code length. */
	DWRF_U8(0); /* Augmentation data. */
	/* Registers saved in CFRAME. */
	#ifdef __x86_64__
	DWRF_U8(DWRF_CFA_advance_loc \| 4);
	DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16);
	DWRF_U8(DWRF_CFA_advance_loc \| 6);
	DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(8);
	/* Extra registers saved for JIT-compiled code. */
	#elif defined(__aarch64__) && defined(__AARCH64EL__) && !defined(__ILP32__)
	DWRF_U8(DWRF_CFA_advance_loc \| 1);
	DWRF_U8(DWRF_CFA_def_cfa_offset); DWRF_UV(16);
	DWRF_U8(DWRF_CFA_offset \| 29); DWRF_UV(2);
	DWRF_U8(DWRF_CFA_offset \| 30); DWRF_UV(1);
	DWRF_U8(DWRF_CFA_advance_loc \| 3);
	DWRF_U8(DWRF_CFA_offset \| -(64 - 29));
	DWRF_U8(DWRF_CFA_offset \| -(64 - 30));
	DWRF_U8(DWRF_CFA_def_cfa_offset);
	DWRF_UV(0);

Uh oh!

gh-128605: Add branch protections for x86_64 in asm_trampoline.S #128606

gh-128605: Add branch protections for x86_64 in asm_trampoline.S #128606

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!