Re: [PATCH 3/7] arm64: uaccess: Add additional userspace GCS accessors

[Jeremy Linton <jeremy.linton@arm.com>]

Hi,

On 3/19/25 8:24 AM, Mark Brown wrote:
> On Tue, Mar 18, 2025 at 03:48:37PM -0500, Jeremy Linton wrote:
> 
>> +static inline u64 load_user_gcs(unsigned long __user *addr, int *err)
>> +{
>> +	unsigned long ret;
>> +	u64 load;
>> +
>> +	if (!access_ok((char __user *)addr, sizeof(load))) {
>> +		*err = -EFAULT;
>> +		return 0;
>> +	}
>> +
>> +	gcsb_dsync();
>> +	ret = copy_from_user(&load, addr, sizeof(load));
>> +	if (ret != 0)
>> +		*err = ret;
>> +	return load;
>> +}
> 
> A GCS load done by the hardware will verify that we are loading from GCS
> memory (the accesses are marked as AccessType_GCS in the pseudocode
> which is then validated in for example S1CheckPermissions()).  Sadly
> there's no equivalent of GCSSTR so we'd need to do the permission check
> ourselves to match this behaviour.

Right, except that if I grab the VMA as a placeholder for the page, 
check to see if its a VM_SHADOW_STACK under any of 
map_read_lock()/lock_vma_under_rcu()/etc and then perform the access, 
the resulting possible fault will have problems with vma locking. 
Otherwise there ends up being a few different races that at the moment 
I've not yet figured out how to fix without making a big mess. For 
example, we can reduce that possible window, by reading the 
value/locking and checking shadow stack state/dropping the 
lock/rereading the value, or some other construct but it seems pointless 
because the suggested problem is that we might be creating a way to 
bypass some of the shadow stack security. In which case, leaving a 
little race is likely the same as leaving it wide open.


Otherwise, maybe we can ignore the problem, or just refuse to allow 
probes on 'RET' instructions which seems to be the main problematic 
case. Although, given we don't really know if GCS is enabled until the 
probe is hit, SIGSEG'ing the target process is a big hammer.

Ignoring it might be a valid option. I guess it could to be one of those 
"if the user puts a uprobe on a RET some of the shadow stack security is 
reduced" footguns. If an attacker can also manipulate the address space 
in a way to exploit it then its probably game over anyway. Ideally, the 
kernel would warn on this, but per the conversation around patch 6/7 
that seems to be off the table.