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 common abstractions for x86 Segments #258

Merged
merged 10 commits into from
Jun 14, 2021
Merged

Add common abstractions for x86 Segments #258

merged 10 commits into from
Jun 14, 2021

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented May 29, 2021

When working on #257 I noticed it might be a good idea to have a common trait for the FS and GS segments, to make their use in things like wrapper structs easier. In fact, it makes sense to have a (different) common trait for all of the segments.

This change introduces the following traits:

  • Segment
    • allows reading/writing the segment register
    • implemented by CS, DS, ES, SS, FS, and GS
  • Segment64
    • allows reading/writing the segment base (using VirtAddr instead of u64)
    • associated const for the Msr
    • implemented by FS and GS
    • this trait will eventually also have the read_u64/write_u64 methods from WIP: FS/GS-based TLS access abstraction #257

Now that the code has a similar structure for all segments, we can use macros to implement the traits.

Also, this change adds some additional documentation about the segments and segment registers.

Outstanding questions:

  • I deprecated all the free functions (rdfsbase()/wrgsbase()/set_ds()/set_cs()/cs() etc...). Does this seem reasonable? We could also wait to deprecate them until 0.15 (with removal in 0.16 or later).
  • Right now both the Segment and Segment64 traits just have static methods. Should I make these be normal methods? It doesn't really matter as the segment structs are singletons (CS::get_reg() vs CS.get_reg()). One advantage of normal methods is that the trait is then object safe, allowing users to use &dyn Segment and &dyn Segment64.

@josephlr josephlr requested a review from phil-opp May 29, 2021 10:53
@josephlr josephlr force-pushed the seg branch 2 times, most recently from 3ee1f21 to 1929da3 Compare May 29, 2021 11:06
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Looks good to me overall! I left a few minor comments inline.

src/instructions/segmentation.rs Show resolved Hide resolved
src/instructions/segmentation.rs Outdated Show resolved Hide resolved
src/instructions/segmentation.rs Outdated Show resolved Hide resolved
src/instructions/segmentation.rs Outdated Show resolved Hide resolved
src/instructions/segmentation.rs Outdated Show resolved Hide resolved
src/instructions/segmentation.rs Show resolved Hide resolved
This function only throws a #UD, which we generally consider to be safe.
Also, add an `Exceptions` section to the `Segment64` docs (this is
similar to the `Panic` section in normal Rust docs).

Signed-off-by: Joe Richey <[email protected]>
Signed-off-by: Joe Richey <[email protected]>
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

The updates all look good to me.

@josephlr josephlr merged commit a122296 into master Jun 14, 2021
@josephlr josephlr deleted the seg branch June 14, 2021 12: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.

2 participants