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

[Sean Christopherson]

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.

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/

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