linux-kernel - Re: [PATCH v4 4/8] arm64: uaccess: Add additional userspace GCS accessors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2a6e1c4b-e8b0-49e2-896c-65c55103b017@arm.com>
Date: Wed, 23 Jul 2025 12:14:17 -0500
From: Jeremy Linton <jeremy.linton@....com>
To: Catalin Marinas <catalin.marinas@....com>
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, broonie@...nel.org, yury.khrustalev@....com,
 kristina.martsenko@....com, liaochang1@...wei.com, will@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 4/8] arm64: uaccess: Add additional userspace GCS
 accessors

Hi,


Thanks for looking at this.

On 7/23/25 4:50 AM, Catalin Marinas wrote:
> On Fri, Jul 18, 2025 at 11:37:36PM -0500, Jeremy Linton wrote:
>> +/*
>> + * Unlike put_user_gcs() above, the use of copy_from_user() may provide
>> + * an opening for non GCS pages to be used to source data. Therefore this
>> + * should only be used in contexts where that is acceptable.
>> + */
> 
> Even in user space, the GCS pages can be read with normal loads, so
> already usable as a data source if one wants to (not that it's of much
> use). So not sure the comment here is needed.

Right, but userspace isn't using it in a privileged context to emulate 
operations that have a permission check performed as part of the read 
when performed by the HW.

This comment was added in V2 following a number of conversations about 
whether this was an actual risk or something that is only a problem if a 
long set of pre-conditions hold true. Conditions which can be summarized 
as "it is too late anyway".

Hence the comment to remind people that this routine isn't assuring the 
page is correctly marked.

I will reword it a bit if that is ok.


> 
>> +static inline u64 load_user_gcs(unsigned long __user *addr, int *err)
> 
> Nitpick: name this get_user_gcs() for symmetry with put_user_gcs().
> 
>> +{
>> +	unsigned long ret;
>> +	u64 load = 0;
>> +
>> +	gcsb_dsync();
> 
> Might be worth a comment here, see the one in gcs_restore_signal().

Sure,

> 
>> +	ret = copy_from_user(&load, addr, sizeof(load));
>> +	if (ret != 0)
>> +		*err = ret;
>> +	return load;
>> +}
> 
> Otherwise the patch looks fine:
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@....com>