Skip to content

Conversation

@jonwoodruff
Copy link
Contributor

This is a new section to compare with recent x86 security extensions. Some extensions may be missing, and bits of these sections may be missing as well.

@jonwoodruff jonwoodruff requested a review from bsdjhb January 4, 2023 10:33
Copy link
Collaborator

@bsdjhb bsdjhb left a comment

Choose a reason for hiding this comment

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

This generally seems fine to me. I had thought that the ENDBRANCH part of CET was a subset of what sentries provide (such that you could dispense with that part of CET if desired). In particular, IIRC CET has some somewhat convoluted ways to handle exceptions (branches that should still be allowed without using ENDBRANCH), and that the management of sentries gives an easier way to permit policy around which branches are protected.

Related to this last is that it may make sense (we should at least add a rationale note?) to have a way to require branches to use sentries, at least for user mode (e.g. via a bit in CR4 that supervisor mode could set).

@bsdjhb
Copy link
Collaborator

bsdjhb commented Mar 9, 2023

Oh, it would probably be good to collapse the history of this branch down and use git rebase (instead of git merge) to simplify the history.

@jrtc27
Copy link
Member

jrtc27 commented Mar 9, 2023

Requiring branch targets to always be sentries would add a pointless instruction for jump tables; the current lowering for CHERI-RISC-V and Morello translated to CHERI-X86-64 (assuming code model with max 2 GiB code) would be something like:

foo:
.Lfoo$jump_table_base:
	...
	leac	.LJTIm_n(%cip), %ctable
	movslq	(%ctable,%rindex,4), %roffset
	leac	.Lfoo$jump_table_base(%cip), %ctarget
	addc	%roffset, %ctarget
	jmpc	*%ctarget

Requiring sentries would put a csealentry %ctarget in between the last two instruction which just bloats things.

@jrtc27
Copy link
Member

jrtc27 commented Mar 9, 2023

Although doing leac .LJTIm_n(%cip), %ctable rather than movc .LJTIm_n@GOTPCREL(%cip), %ctable is a bit dodgy actually, if there are cases the compiler believes are impossible but end up happening, which one can easily construct... then you get an untrapped OOB read that is then used as an offset to jump to. Should fix CHERI-RISC-V and Morello to be more conservative...

@brooksdavis
Copy link
Member

This PR would benefit from rebasing.

@bsdjhb bsdjhb force-pushed the x86_security_extensions branch from f10349e to c4c9757 Compare July 27, 2023 15:50
If the \insnxesref{XCHG} instructions \texttt{91} - \texttt{97} are not
commonly used, they could be removed in capability mode.

\section{Relationship to x86 Security Extensions}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add "Existing" before x86 perhaps?

presumably enabled for both consumer and professional variants.

In userspace, a thread may write its PKRU register, which maps
16 pairs of read/write permissions coorosponding to the 16 possible
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
16 pairs of read/write permissions coorosponding to the 16 possible
16 pairs of read/write permissions corresponding to the 16 possible

By writing this register, a thread may quickly remove access to subsets
of pages.
Current implementations appear to flush the back end of the pipeline,
incuring a cycle penalty of roughly 20 cycles.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
incuring a cycle penalty of roughly 20 cycles.
incurring a cycle penalty of roughly 20 cycles.

of pages.
Current implementations appear to flush the back end of the pipeline,
incuring a cycle penalty of roughly 20 cycles.
This filtering is naturally per-thread, unlike the page table iteself
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
This filtering is naturally per-thread, unlike the page table iteself
This filtering is naturally per-thread, unlike the page table itself


While MPK appears to be extremely unscalable on the surface, work
has been done to virtualise the page sets with an API similar to
memprotect so that sets of pages can have their access permissions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
memprotect so that sets of pages can have their access permissions
mprotect so that sets of pages can have their access permissions

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if you intend a specific memprotect() function distinct from the POSIX mprotect()? If so, this might need a citation?

As a result, MPX "failed open" in many cases where unexpected
cases arose.
If the pointer from the bounds table did not match the pointer
from memory, bounds were left in state that would not fault.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from memory, bounds were left in state that would not fault.
from memory, bounds were left in a state that would not fault.

CHERI relies on simple invariants that can be enforced within
single-cycle instructions.
Intel SGX instead trusts the correctness and atomicity of
hyper-visor-like subroutine calls due to their being
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
hyper-visor-like subroutine calls due to their being
hypervisor-like subroutine calls due to their being

hyper-visor-like subroutine calls due to their being
provided by processor firmware.

On one hand, the SGX precedent suggests that trusted hyper-visor-like
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
On one hand, the SGX precedent suggests that trusted hyper-visor-like
On one hand, the SGX precedent suggests that trusted hypervisor-like

or branch prediction can easily push overheads over 20\% and
greatly restrict adoption.

\subsection{Intel SGX (deprecated)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think SGX is deprecated fully (unlike MPX). Intel has dropped it from "consumer" CPUs, but Xeons still ship it I believe according to https://community.intel.com/t5/Blogs/Products-and-Solutions/Security/Rising-to-the-Challenge-Data-Security-with-Intel-Confidential/post/1353141


On one hand, the SGX precedent suggests that trusted hyper-visor-like
subroutines can be implemented in the CPU.
On the other hand, Intel SGX was only implemented in a single Intel
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure these next two sentences are accurate. Intel's current top-of-line Xeon processors ship SGX: https://www.intel.com/content/www/us/en/products/details/processors/xeon/scalable/platinum.html

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.

5 participants