Skip to content
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.

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

Distinguish between function pointers and code pointers through additional relocation types #306

Merged
merged 1 commit into from
Feb 13, 2025

Conversation

dpgao
Copy link
Contributor

@dpgao dpgao commented Jan 30, 2025

First cut of a proposal to distinguish between function pointers and code pointers through additional relocation types.

Previously, function pointers and code pointers (e.g., those used for C++ exception landing pads) are relocated in the same way. This PR introduces new relocation types to distinguish between them to help library-based compartmentalisation.

Function pointers continue to use R_MORELLO_CAPINIT and code pointers use the new R_MORELLO_CODE_CAPINIT relocation. Use R_{AARCH64,MORELLO}_FUNC_RELATIVE for function pointers that need to be relatively relocated, whilst code pointers and integer values continue to use R_{AARCH64,MORELLO}_RELATIVE.

@@ -476,6 +476,11 @@ The following nomenclature is used in the descriptions of relocation operations:
the second entry holds a platform-specific offset or pointer. The pair of
pointer-sized entries will be relocated with ``R_MORELLO_TLSDESC(S+A)``.

- ``Delta(S)`` if ``S`` is a normal symbol, resolves to the difference between the
Copy link

Choose a reason for hiding this comment

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

This is just a copy of what's in aaelf64.rst, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

seen in intermediate relocatable objects and not in executables or shared
objects because the toolchain should only allow code pointers to be taken for
non-preemptible symbols. These relocations would subsequently be converted to
relative relocations at static-link time.
Copy link

Choose a reason for hiding this comment

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

Do we want to explicitly state that this restriction may be enforced? (Or make it a static relocation, but that would mean we could never backpedal on this if it turns out we need it for preemptible symbols...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put this explanation here mainly so that nobody can get confused over why RTLD doesn't actually handle this relocation type.

How about I change the language a bit to say that the current behaviour of the toolchain is to not allow preemptible symbols, but this may change in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reworked the language.

@smithp35
Copy link
Contributor

smithp35 commented Feb 4, 2025

@jrtc27 I can check over the text as best I can, but don't have a lot of context on the CHERI/Morello side. I'm happy to take your lead on whether this is good to merge. When you're happy with it, please approve it and I'll be able to merge.

@jrtc27
Copy link

jrtc27 commented Feb 4, 2025

@jrtc27 I can check over the text as best I can, but don't have a lot of context on the CHERI/Morello side. I'm happy to take your lead on whether this is good to merge. When you're happy with it, please approve it and I'll be able to merge.

Ok, thanks, I'll work with Dapeng to get the text refined and approve when ready.

For some context if it helps, the design has come out of various discussions over the past 6 months between Dapeng, myself and a few other colleagues. He's been doing the toolchain and runtime implementation work and we're working to deploy it to all CheriBSD users. Based on rebuilding the entire OS and ~10k package set, and running a KDE-based graphical desktop stack (so all kinds of function pointer-related C++ features being exercised to find corner cases, which they have in the past; exceptions, vtables, even GNU C computed goto have all shown up there over the course of this work), we believe these relocations are what we need, so the next step is to get it in the hands of users and see if there's anything we've overlooked.

Comment on lines 779 to 782
compartmentalization (c18n). Note that current toolchain only allows code
pointers to be taken for non-preemptible symbols, so all instances of
``R_MORELLO_CODE_CAPINIT`` would be converted to relative relocations at
static-link time. This behaviour is not guaranteed in the future.
Copy link

Choose a reason for hiding this comment

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

Suggested change
compartmentalization (c18n). Note that current toolchain only allows code
pointers to be taken for non-preemptible symbols, so all instances of
``R_MORELLO_CODE_CAPINIT`` would be converted to relative relocations at
static-link time. This behaviour is not guaranteed in the future.
compartmentalization (c18n). Currently for security reasons this relocation may only be used for non-preemptible symbols, and thus will be converted to ``R_MORELLO_RELATIVE`` at link time, making it in effect a static relocation. However, this restriction may be removed in future if needed, and is therefore classed as a dynamic relocation.

Also, is it an error to use this relocation against a non-STT_FUNC symbol? If so we should state that. (Think carefully about STT_GNU_IFUNC, not just data/untyped symbols)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not an error to use this for a non-STT_FUNC symbol, and the toolchain will just convert it to a relative relocation (or error out for global symbols).

Copy link

Choose a reason for hiding this comment

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

I'll rephrase: is there a legitimate use case for permitting such uses, or should they be forbidden by the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably forbid it - I cannot think of any legitimate use cases.

@dpgao dpgao requested a review from jrtc27 February 5, 2025 22:21
@dpgao dpgao requested a review from jrtc27 February 5, 2025 22:59
Copy link

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of wording tweaks

@dpgao
Copy link
Contributor Author

dpgao commented Feb 13, 2025

@smithp35 We are ready to merge this now. Thanks!

@smithp35
Copy link
Contributor

OK. I've given it a read through myself, no objections. Will merge.

@smithp35 smithp35 merged commit 92cd879 into ARM-software:main Feb 13, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants