[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e8ada81d-0e10-412b-8792-035bc63113a6@arm.com>
Date: Fri, 21 Mar 2025 18:43:23 -0500
From: Jeremy Linton <jeremy.linton@....com>
To: Mark Brown <broonie@...nel.org>
Cc: linux-trace-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
mhiramat@...nel.org, oleg@...hat.com, peterz@...radead.org, acme@...nel.org,
namhyung@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org, irogers@...gle.com,
adrian.hunter@...el.com, kan.liang@...ux.intel.com,
thiago.bauermann@...aro.org, yury.khrustalev@....com,
kristina.martsenko@....com, liaochang1@...wei.com, catalin.marinas@....com,
will@...nel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/7] arm64: uaccess: Add additional userspace GCS
accessors
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.
Powered by blists - more mailing lists