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

Add program property #428

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Add program property #428

merged 2 commits into from
Aug 2, 2024

Conversation

kito-cheng
Copy link
Collaborator

We introduce .note.gnu.property section to store infomations that linker or runtime system may use, and we define 4 program property classes to specifying the merge semantics, it's used for forward compatibility in linker implementations, allowing linker can correctly handle program property types even if they are not recognized.

We don't define any program property within this PR, it would be separate PR like #417

@sorear
Copy link
Collaborator

sorear commented Feb 27, 2024

Good feature to add.

Might want to make clear that there can be other GNU properties not defined by RISC-V outside the LOPROC..HIPROC range, and RISC-V might define other properties in the future besides the bitmasks. Actually I am half tempted to obsolete .riscv.attributes in favor of an architecture-neutral solution, since GNU properties can have string values, not just integers.

x86 has rather precise rules for what it means if a bitmask is missing, and when a linker needs to delete the bitmasks. We should probably copy those unless we have a specific reason not to. Would a review with specific text help here?

@kito-cheng
Copy link
Collaborator Author

Actually I am half tempted to obsolete .riscv.attributes in favor of an architecture-neutral solution, since GNU properties can have string values, not just integers.

I am little incline to deprecate that before, but I didn't find immediate reason to doing so yet, my thought is we may still add information into .riscv.attributes, but only limited to those info we think it won't need to use at runtime (e.g. loader or dynamic linker), and consider it as strip-able (yeah, we don't have clear rule on that yet.), and put information into program property if we may use that at runtime.

x86 has rather precise rules for what it means if a bitmask is missing, and when a linker needs to delete the bitmasks. We should probably copy those unless we have a specific reason not to. Would a review with specific text help here?

Suggestion with concrete diff is very welcome to me :P

@lenary
Copy link
Contributor

lenary commented Jun 7, 2024

I'm against this "generic" approach - I found that, for a similar whole-process security-focussed property, you want the linker to have three behaviours for each specific bit (this example only talks about AND-style properties, which are the kind that really matters for security):

  • Forcing a bit to be "On", for testing (before all your objects have the bit set)
  • Forcing a bit to be "Off", so you have control over the exact time you mark an executable/library as supporting/requiring an OS feature, even if all the objects in that image already have their bit set (this is really important for whole-process security-focussed properties that cross shared library boundaries, especially with dlopen)
  • Following the automatic behaviour (for when people are building newer projects correctly)

If we include the automatic behaviour here for bits with unknown semantics, then it becomes annoyingly difficult to force the bit to be off without recompiling all your objects to remove the marking, which usually brings in other code generation churn. This is especially the case if the linker is also going to use the computed property to choose e.g. a PLT style that should be used.

I think it's reasonable to define the bits/groups as we go, though specifying some standard semantics for the bits (AND/OR) seems good, it's applying those semantics to unknown bits that I'm unhappy with.

@kito-cheng
Copy link
Collaborator Author

@lenary What if we separate it into two group? security group and non-security group, the bit within security group will error out if got unknown bit, which should applied on some thing like CFI, and non-security group will keep doing AND/OR semantics on unknown bits.

The reason I would like keep the non-security group is we always has some transition period when adding new bits, which will cause older toolchain release not work well with newer toolchain, one recent piratical example is the atomic ABI attribute*...LLVM has support that, but binutils isn't and then...got crashed (but that is not intend to crash actaully).

@lenary
Copy link
Contributor

lenary commented Jul 5, 2024

I'm just going to talk about AND fields for the moment, because I'm not sure I've ever really seen OR fields in the wild. I'd appreciate links if people have seen these.

Honestly, I think this goes too general too quickly. I think the right approach is to define that bits the linker knows about should be ANDed by default in any AND field, but that linkers have the right to override this behaviour for specific bits. The linker should be allowed/encouraged to clear any bits it doesn't know about (and should definitely not error if it sees them). I think this is fine because almost all deployments of features that need markings, that i've seen, tend to need coordinated modifications across a whole toolchain, from the compiler and linker to the libraries, and often into the OS too. For this reason, it's really hard to make any good choices around mixing tools that support and don't support these features.

It sounds like the binutils crash about the atomics ABI was just a bug, rather than anything more, and an explicit requirement to ignore (and clear?) any unknown bits would have helped here.

We could suggest a warning when a linker does come across a bit it doesn't know about, BUT it's not clear how actionable that would be, and it would also not much help with the older linker versions.

We introduce .note.gnu.property section to store infomations that linker
or runtime system may use, and we define 4 program property classes to
specifying the merge semantics, it's used for forward compatibility in linker
implementations, allowing linker can correctly handle program property types
even if they are not recognized.

We don't define any program property within this PR, it would be
separate PR like #417
@kito-cheng
Copy link
Collaborator Author

@lenary Yeah, I think let defer those generic rule later, and add that when needed, I agree that it will cause problem is linker should really take any action when bit is set, like PLT need generate special PLT.

@kito-cheng
Copy link
Collaborator Author

Changes:

  • Drop generic rule, and keep the description for program property only.

@kito-cheng kito-cheng merged commit 170103b into master Aug 2, 2024
4 checks passed
@kito-cheng kito-cheng deleted the prog-prop branch August 2, 2024 06:48
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