[RFC PATCH v3 4/4] x86/sgx: Implement SGX specific hooks in SELinux

[Xing, Cedric]

On 7/8/2019 6:33 PM, Sean Christopherson wrote:
> On Sun, Jul 07, 2019 at 04:41:34PM -0700, Cedric Xing wrote:
>> +static int enclave_mprotect(struct vm_area_struct *vma, size_t prot)
>> +{
>> +	struct ema_map *m;
>> +	int rc;
>> +
>> +	/* is vma an enclave vma ? */
>> +	if (!vma->vm_file)
>> +		return 0;
>> +	m = ema_get_map(vma->vm_file);
>> +	if (!m)
>> +		return 0;
>> +
>> +	/* WX requires EXECMEM */
>> +	if ((prot && PROT_WRITE) && (prot & PROT_EXEC)) {
>> +		rc = avc_has_perm(&selinux_state, current_sid(), current_sid(),
>> +				  SECCLASS_PROCESS, PROCESS__EXECMEM, NULL);
>> +		if (rc)
>> +			return rc;
>> +	}
>> +
>> +	rc = ema_lock_map(m);
>> +	if (rc)
>> +		return rc;
>> +
>> +	if ((prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC))
>> +		rc = ema_apply_to_range(m, vma->vm_start, vma->vm_end,
>> +					ema__chk_X_cb, vma->vm_file);
>> +	if (!rc && (prot & PROT_WRITE) && !(vma->vm_flags & VM_WRITE))
>> +		rc = ema_apply_to_range(m, vma->vm_start, vma->vm_end,
>> +					ema__set_M_cb, NULL);
> 
> Not tracking whether a page has been mapped X and having ema__chk_W_cb()
> allows an application to circumvent W^X policies by spinning up a helper
> process.

See my response in another email.

This problem has nothing to do with the architecture, but is just a 
policy choice. Your patch of EXECDIRTY is another possible policy, by 
combining (or *not* distinguishing) W->X and X->W into a single WX 
"maximal protection".

> Ignoring that issue, this approach suffers from the same race condition I
> pointed out a while back[1].  If process A maps a page W and process B
> maps the same page X, then the result of ema__chk_X_cb() depends on the
> order of mprotect() calls between A and B.
> 
> [1] https://lore.kernel.org/linux-security-module/20190614200123.GA32570@linux.intel.com/

You seem to be talking about the same problem in both places.

>> +	ema_unlock_map(m);
>> +
>> +	return rc;
>> +}