Re: [RFC PATCH v2 2/5] x86/sgx: Require userspace to define enclave pages' protection bits

[Jarkko Sakkinen]

On Wed, Jun 05, 2019 at 07:11:42PM -0700, Sean Christopherson wrote:
> [SNAP]

Same general criticism as for the previous patch: try to say things as
they are without anything extra.

> A third alternative would be to pull the protection bits from the page's
> SECINFO, i.e. make decisions based on the protections enforced by
> hardware.  However, with SGX2, userspace can extend the hardware-
> enforced protections via ENCLU[EMODPE], e.g. can add a page as RW and
> later convert it to RX.  With SGX2, making a decision based on the
> initial protections would either create a security hole or force SGX to
> dynamically track "dirty" pages (see first alternative above).
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>

'flags' should would renamed as 'secinfo_flags_mask' even if the name is
longish. It would use the same values as the SECINFO flags. The field in
struct sgx_encl_page should have the same name. That would express
exactly relation between SECINFO and the new field. I would have never
asked on last iteration why SECINFO is not enough with a better naming.

The same field can be also used to cage page type to a subset of values.

/Jarkko