8000 Support relro in delocator by torben-hansen · Pull Request #2455 · aws/aws-lc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support relro in delocator #2455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jun 3, 2025
Merged

Support relro in delocator #2455

merged 5 commits into from
Jun 3, 2025

Conversation

torben-hansen
Copy link
Contributor
@torben-hansen torben-hansen commented Jun 2, 2025 •
edited
Loading

Issues:

CryptoAlg-2491

Description of changes:

This PR adds support for relocation read-only (relro) sections .data.rel.ro[.local] in the delocator. These type of sections are mapped as writable when the binary is loaded into memory. But after the relocation phase, permissions for the memory pages are re-configured to read-only. This doesn't work for the static FIPS build. In fact, all .data sections are fatal for the static FIPS build. relro section usage seems to have increased for newer compilers e.g. gcc14. I don't know why though. Looking at the assembly it looks related to vector optimisations and never happens for -O (strictly) below 2. I wasn't able to reverse engineer conditions from gcc though.

A relro section takes the form

	.section .data.rel.ro.local
	.align 8
.LC01:
	.quad foo
.LC02:
	.quad foofoo

The local symbol maps to another symbol (function symbol). For example, .LC01 maps to foo, in the sense that during relocation, the address of the function implementation foo can be referenced using the local symbol .LC01. The local symbol value (in this case the function address) is loaded using IP-addressing. This typically takes the form:

	movq  %rdx, %xmm1
	movq .L00(%rip), %xmm0
	punpcklqdq  %xmm1, %xmm0

The strategy to support relro is as follows:

  • Identify local symbol <-> function symbol mappings under relro sections.
  • Locate (move-based) statements where local symbols from relro sections are used to load function symbol addresses and rewrite using a lea.
  • Delete relro sections

A transform example is as follows. Assume we have already located the mapping .LC01 <-> foo. This is simply parsing related. Then assume we have the following load (the problematic part of the middle statement):

	movq  %rdx, %xmm1
	movq .L00(%rip), %xmm0
	punpcklqdq  %xmm1, %xmm0

The delocator already issues a local target for foo, with the name .Lfoo_local_target, that we will use for the rewrite. For a lea instruction, we need to load the result into a non-vector register. We will use a caller-saved register, say rax. Hence, the first rewrite is:

	movq  %rdx, %xmm1
	leaq	.Lfoo_local_target(%rip), %rax
	movq	%rax, %xmm0
	punpcklqdq  %xmm1, %xmm0

Correctness should be maintained because xmm0 contains the same value as before. This almost works. But even though rax is caller-saved, we have no idea whether it's used anywhere else under the function scope. Hence, to maintain correctness, we must push rax to the stack and restore it after the move instruction. Hence, the final rewrite is

	movq  %rdx, %xmm1
	leaq -128(%rsp), %rsp
	pushq %rax
	leaq	.Lfoo_local_target(%rip), %rax
	movq	%rax, %xmm0
	popq %rax
	leaq 128(%rsp), %rsp
	punpcklqdq  %xmm1, %xmm0

(This also handles clearing any red zones). The push-pop stack to mantain correctness under a lea re-write is a known technique in the delocator.

Deleting relro sections is also just a game of identifying the correct patterns to remove. One fun little complexity to handle is the assembly macros .set because they can alias local symbols that we have already mapped to a function symbol. These are also handled.

Call-outs:

Pushing and popping from the stack cost a tiny number of CPU cycles. But luckily, the relro sections are only (until now at least) used to populate the .bss allocated virtual function tables that is a once-only, lazy operation. So, I expect no performance impact at all. In fact, I measured and saw none in my little test.

Testing:

Added unit tests for the new feature in the delocator to preserve functionality. Existing tests in the CI will cover correctness.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@torben-hansen torben-hansen requested a review from a team as a code owner June 2, 2025 21:35
torben-hansen
torben-hansen commented Jun 2, 2025
View reviewed changes
util/fipstools/delocate/delocate.go
@@ -722,7 +761,7 @@ func (d *delocation) processAarch64Instruction(statement, instruction *node32) (
/* ppc64le

[PABI]: “64-Bit ELF V2 ABI Specification. Power Architecture.” March 21st,
2017
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are because I converted space indentation to tab indentation.

torben-hansen
torben-hansen commented Jun 2, 2025
View reviewed changes
torben-hansen
torben-hansen commented Jun 2, 2025
View reviewed changes
@codecov-commenter
Copy link
codecov-commenter commented Jun 2, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.88%. Comparing base (4e97131) to head (cfdd278).
Report is 14 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2455      +/-   ##
==========================================
- Coverage   78.89%   78.88%   -0.02%     
==========================================
  Files         621      621              
  Lines      108613   108675      +62     
  Branches    15414    15421       +7     
==========================================
+ Hits        85685    85723      +38     
- Misses      22258    22281      +23     
- Partials      670      671       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

skmcgrail
skmcgrail previously approved these changes Jun 3, 2025
View reviewed changes
dkostic
dkostic reviewed Jun 3, 2025
View reviewed changes
util/fipstools/delocate/delocate.go Outdated
symbol = localTargetName(d.relroLocalLabelToFuncMap[symbol])

// Transform the opcode and arguments
instructionName = "leaq"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused?

< 8000 div data-view-component="true" class="TimelineItem-badge">
@torben-hansen torben-hansen enabled auto-merge (squash) June 3, 2025 17:25
@torben-hansen torben-hansen merged commit fd7556f into aws:main Jun 3, 2025
116 of 118 checks passed
This was referenced Jun 3, 2025
Closed
Open
@justsmth justsmth mentioned this pull request Jun 6, 2025
Merged
justsmth added a commit that referenced this pull request Jun 13, 2025
@justsmth
Prepare v1.53.0 (#2471)
0f76ff1 
## What's Changed
* Add build with hardened flag by @m271828 in
#2396
* Openssl tool output ordered by options provided by @justsmth in
#2452
* [SCRUTINICE] Remove redundant condition check by @nhatnghiho in
#2450
* Support relro in delocator by @torben-hansen in
#2455
* Explicitly don't allow buffers aliasing in ctr-drbg implementation by
@torben-hansen in #2458
* Remove unused Wi
92BF
ndows afunix.h by @justsmth in
#2461
* Revert "Rework memory BIOs and implement BIO_seek (2nd try) (#2433)"
by @justsmth in #2466
* Use max_cert_list for TLSv1.3 NewSessionTicket by @skmcgrail in
#2453
* ML-KEM memory safety by @m271828 in
#2263
* Simplify Compiler CI jobs by @justsmth in
#2430
* Improve support for multilib-style distros in our test scripts by
@justsmth in #2467
* Fix Ruby mainline and nginx CI by @samuel40791765 in
#2460
* Add hardened build back in by @m271828 in
#2474
* Fix OCSP integration test failures by @samuel40791765 in
#2480
* Fix some theoretical missing earlyclobber markers in inline assembly
by @torben-hansen in #2477
* Simplify sshkdf and kbkdf by @torben-hansen in
#2478
* Run 3p module tests on python 3.13, add patch for 3.14 by
@WillChilds-Klein in #2476
* [UPSTREAM] Fix BIO_eof for BIO pairs by @justsmth in
#2440
* Fix service indicator in HKDF, more paranoid zeroization, and simplify
logic by @torben-hansen in #2482


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skmcgrail skmcgrail skmcgrail approved these changes

@dkostic dkostic dkostic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@torben-hansen @codecov-commenter @skmcgrail @dkostic
0