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

Make .text execute-only; add tests #450

Merged
merged 1 commit into from
Nov 4, 2022
Merged

Conversation

adamsteen
Copy link
Contributor

Now for my next trick, execute only Solo5 unikernels?

I wish i could test this, but my hardware is just too old.

Thoughts on taking this forward?

Note: this is dependant on #447 add openbsd mprotect ept support, and would need to be rebased when that went in.

@adamsteen adamsteen force-pushed the xnor branch 2 times, most recently from f1f5462 to 11895ee Compare April 26, 2020 23:46
@mato mato added enhancement host/openbsd Applicable to OpenBSD hosts pr/rfc Work in progress or RFC, do not merge target/hvt Applicable to hvt target labels Apr 28, 2020
Copy link
Member

@mato mato left a comment

Choose a reason for hiding this comment

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

I'll have a look in a bit more detail in due course, but you're on the right track here. It's awesome that #447 and VMM_IOC_MPROTECT_EPT on OpenBSD enable execute-only page mappings in the guest!

When you say you can't test this due to old hardware, why is that? ISTR that anything with EPT can do execute-only, but I might be wrong...

FLAGS values come from PF_x in elf.h */
rodata PT_LOAD FLAGS(5);
Copy link
Member

Choose a reason for hiding this comment

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

I think you want 4 here (PF_R only).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

your right thank you.

*
* Guest-side page protections:
*
* Manipulating guest-side (EPT) mappings is currently not supported by
* FreeBSD vmm, so there is nothing more we can do.
*/
prot &= ~(PROT_EXEC);
prot |= PROT_READ;
Copy link
Member

Choose a reason for hiding this comment

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

This (and the other host variants) are a bit hard to read now. I'd make it explicit that we're "squashing" PROT_EXEC into PROT_READ on the host side, and nothing else:

if (prot & PROT_EXEC) {
    prot &= ~(PROT_EXEC);
    prot |= PROT_READ;
}

This makes the intent crystal clear, i.e.

  • PROT_READ: (no change)
  • PROT_WRITE: (no change)
  • PROT_EXEC: -> PROT_READ

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first just reading this, i was unsure, but i thought i would implement it and then reread the code, and its much more readable.

@mato mato changed the title Make texts section execute only and add xnor and rnow tests (not enalbled atm) Make .text execute-only; add tests Apr 28, 2020
@adamsteen
Copy link
Contributor Author

When you say you can't test this due to old hardware, why is that? ISTR that anything with EPT can do execute-only, but I might be wrong...

Intel® 64 and IA-32 Architectures Software Developer's Manual Combined Volumes 3A, 3B, 3C, and 3D: System Programming Guide (Page 1727)

A.10 VPID AND EPT CAPABILITIES
 - If bit 0 is read as 1, the processor supports execute-only translations by EPT. This support allows software to configure EPT paging-structure entries in which bits 1:0 are clear (indicating that data accesses are not allowed) and bit 2 is set (indicating that instruction fetches are allowed).

Bit 0 does not read 1 on the hardware i have access to, so i am unable to test execute only.

@adamsteen
Copy link
Contributor Author

this PR is ready for review and merging.

I have been able to run the tests and all tests expected to pass on OpenBSD do.

probably want to merge #495 first.

@adamsteen
Copy link
Contributor Author

Please note this works perfectly for the latest release OpenBSD 6.9

@adamsteen adamsteen marked this pull request as draft December 17, 2021 12:19
@adamsteen adamsteen marked this pull request as ready for review December 17, 2021 12:31
@dinosaure
Copy link
Collaborator

OpenBSD 6.8 fails, it's expected?

@adamsteen
Copy link
Contributor Author

Yes it won’t work on OpenBSD 6.8 or 6.9

I had it working for 6.8 but someone broke the kernel side for 6.9 then I got it restored for 7.0. I only run current, so testing 7.0 is hard as I don’t have the hardware!

It should work for 7.0 and definitely works for Current!

@adamsteen
Copy link
Contributor Author

OpenBSD 7.1 is out very soon, is there any chance of getting the ci image updated? It should just be a matter of running, sysupgrade and pkg_add -u after each reboot a couple of times! Or following the instructions in the solo5-ci package?

the current OpenBSD image is over 2 years old!

@dinosaure
Copy link
Collaborator

OpenBSD 7.1 is out very soon, is there any chance of getting the ci image updated? It should just be a matter of running, sysupgrade and pkg_add -u after each reboot a couple of times! Or following the instructions in the solo5-ci package?

the current OpenBSD image is over 2 years old!

Unfortunately, I don't have (yet) the control on the CI so it will take a time to upgrade the CI with OpenBSD 7.1. But we should definitely upgrade the CI and it's in my TODO list.

@adamsteen
Copy link
Contributor Author

@dinosaure Thank you for the update, appreciate it.

@adamsteen
Copy link
Contributor Author

@dinosaure Hi, any updates on control of the CI?

@dinosaure
Copy link
Collaborator

/cc @TheLortex recently upgraded the CI, can you test on OpenBSD 7.1?

@adamsteen
Copy link
Contributor Author

adamsteen commented Sep 9, 2022 via email

@TheLortex
Copy link
Collaborator

It's not upgraded yet, still on old OpenBSDs, I have just moved and restarted the CI infrastructure.

@adamsteen
Copy link
Contributor Author

rebased onto masted, and tests pass on OpenBSD 7.2

@dinosaure
Copy link
Collaborator

Let's merge this PR 👍 Thanks for your work.

@dinosaure dinosaure merged commit 54ab80b into Solo5:master Nov 4, 2022
@adamsteen
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement host/openbsd Applicable to OpenBSD hosts pr/rfc Work in progress or RFC, do not merge target/hvt Applicable to hvt target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants