Hi Jarkko,
On 12/4/2021 2:50 PM, Jarkko Sakkinen wrote:
What about:
"x86/sgx: Add encl_page->vm_run_prot_bits for dynamic permission changes"
Sure.
On Wed, Dec 01, 2021 at 11:23:03AM -0800, Reinette Chatre wrote:
Enclave creators declare their paging permission intent at the time
the pages are added to the enclave. These paging permissions are
vetted when pages are added to the enclave and stashed off
(in sgx_encl_page->vm_max_prot_bits) for later comparison with
enclave PTEs.
Current permission support assume that enclave page permissions
remain static for the lifetime of the enclave. This is about to change
with the addition of support for SGX2 where the permissions of enclave
pages belonging to an initialized enclave may be changed during the
enclave's lifetime.
Introduce runtime protection bits in preparation for support of
By writing "Introduce runtime protection bits", instead of simply "Add
encl_page->vm_run_prot_bits", the only thing you are adding is obfuscation.
Try to refer to the "exact thing", instead of English rephrasing
whenever possible.
enclave page permission changes. These bits reflect the active
permissions of an enclave page and are not to exceed the maximum
protection bits that passed scrutiny during enclave creation.
Associate runtime protection bits with each enclave page. Initialize
the runtime protection bits to the vetted maximum protection bits
on page creation. Use the runtime protection bits for any access
checks.
I guess the first sentence in this paragraph is completely redundant
as the first sentence of the previous paragraph tells the exact
same story.
The previous paragraph introduces what these bits are and what they mean
and the second describes how they are used. I can merge the paragraphs.
struct sgx_encl_page hosting this information is maintained for each
enclave page so the space consumed by the struct is important.
The existing vm_max_prot_bits is already unsigned long while only using
three bits. Transition to a bitfield for the two members containing
protection bits.
Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
So this commit message left the most important thing unanswered,
or I missed it (which happens quite often): why two fields instead
of one? Why vm_max_port_bits needs to stay constant?
It's something that should be clearly documented.
vm_max_prot_bits is the vetted EPCM permissions an enclave is allowed to
have (for EADDed pages it is the value from secinfo). Permissions can be
changed using SGX2 but they should never exceed vm_max_prot_bits.
vm_run_prot_bits reflects the current (from OS perspective) active EPCM
permissions and replaces the current usages of vm_max_prot_bits in
runtime (VMA and PTE) permission checks.
Consider this example how vm_max_prot_bits and vm_run_prot_bits are used:
(1) Add enclave page with secinfo of RW to uninitialized enclave
vm_max_prot_bits = RW
vm_run_prot_bits = RW
(2) User space runs SGX_IOC_PAGE_MODP (renamed
to SGX_IOC_ENCLAVE_MOD_PROTECTIONS) to change the permissions to
read-only. This is allowed because vm_max_prot_bits = RW. Now:
vm_max_prot_bits = RW
vm_run_prot_bits = R
At this point only new read-only VMAs would be allowed to access
this page and PTEs would not allow write access ... this is guided
by vm_run_prot_bits.
(3) User space runs SGX_IOC_PAGE_MODP (renamed
to SGX_IOC_ENCLAVE_MOD_PROTECTIONS) to change the permissions to RX.
This will be denied because vm_max_prot_bits = RW.
(3) User space runs SGX_IOC_PAGE_MODP (renamed
to SGX_IOC_ENCLAVE_MOD_PROTECTIONS) to change the permissions to RW.
This will be allowed because vm_max_prot_bits = RW.
If this is helpful I can add this example to the changelog.
Reinette
