[PATCH v8 00/20] aarch64: Add support for Guarded Control Stack extension

[Adhemerval Zanella Netto]

On 16/01/25 11:48, Yury Khrustalev wrote:
> On Thu, Jan 16, 2025 at 11:33:53AM -0300, Adhemerval Zanella Netto wrote:
>>
>> On 16/01/25 10:55, Yury Khrustalev wrote:
>>> On Thu, Jan 16, 2025 at 10:49:45AM -0300, Adhemerval Zanella Netto wrote:
>>>>
>>>> Hi Yuri,
>>>>
>>>> I think I reviewed all the remaning patches, and the only contention I have is
>>>> the inclusion of aarch64_gcs_policy=1 that allows dlopen to disable GCS support
>>>> if a shared library does not contain the GCS marking.
>>>
>>> I think there is some misunderstanding here. If glibc.cpu.aarch64_gcs is set,
>>> then dlopen() can never disable GCS. If policy is non-zero, it will be an error,
>>> and if policy is 0, GCS will remain enabled.
>>
>> The problem is the silent change of program security semantic at *runtime*,
>> where auditing is more complex.  On sysdeps/aarch64/dl-gcs.c:
> 
> OK, this might be a good point but this policy allows gradual deployment. It means,
> when we request GCS via tunable, run application correctly with or without GCS
> depending on the markings. If the first unmarked library is loaded via dlopen(),
> it will be an error.
> 
>>
>> ...
>>
>> For dl_aarch64_gcs_policy=1, it silent changes the dl_aarch64_gcs to 0 which
>> disable future GCS checks.
> 
> It just disables GCS as if tunable was not set from the beginning.
> 
>>
>> In fact, the manual sentence:
>>
>>   GCS policy @code{1} means that GCS is enabled if @code{glibc.cpu.aarch64_gcs}
>>   is set and all binaries are GCS-marked. If GCS is required and an incompatible
>>   library is loaded via @code{dlopen}, it is an error, otherwise any
>>   incompatible binary will disable GCS. 
>>
>> should be improve to state that what is disabled is the GCS *check* for future
>> dlopen, not GCS itself.
> 
> Sorry, this is not correct. What is disabled is GCS itself (making is the same as
> when tunable wasn't set from the start).

Hum, how exactly GCS is disabled once is enabled? Afaiu aarch64_gcs=1 enables
it with PR_SHADOW_STACK_ENABLE on _start (assuming dynamic case), and there
is no other PR_SET_SHADOW_STACK_STATUS command after (so GCS is still enabled
in newly created threads). Kernel documentation is not explicit clear on
how to disable GCS by the thread, but I assume it through PR_SET_SHADOW_STACK_STATUS
with argument 0. 

Am I missing something here?

> 
>> And still think this semantic is far from ideal.
> 
> OK, this may be open to debate, but it would be better if this problem was
> brought up earlier...
> 
>>
>> In fact, two tunables for GCS support is *really* confusing.  For instance 
>> aarch64_gcs_policy=0,aarch64_gcs=1 does not make sense (since aarch64_gcs is
>> ignored if aarch64_gcs_policy is not set).
> 
> Sorry, this is wrong. aarch64_gcs is never inored. It's the other way round:
> aarch64_gcs_policy is irrelevant when aarch64_gcs is not set.

Right, my mistake here; I meant the other way around indeed.

> 
>> Assuming GCS is never disabled once enabled (which seems the intention of this
>> initial patchset), I think we can simplify to just one tunable:
>>
>>   aarch64_gcs=0  -  GCS is disabled and any GCS-marking is ignored.  The default.
>>   aarch64_gcs=1  -  GCS is enabled, and taken as a hint at loading time.  If
>>                     any binary is not GCS-marked, or if kernel does not
>>                     support it, GCS is not enabled. 
>>                     dlopen enforces GCS if it was enabled at loading time,
> 
> I don't understand what you mean by "dlopen enforces GCS".

If GCS is enabled at _start, because the binary and its dependencies are
GCS-marked; dlopen will fail if trying to load a GCS-mark shared library.
If GCS was not enabled, dlopen will ignore the GCS-marking.

> 
>>                     otherwise GCS-marking is ignored.
>>   aarch64_gcs=2  -  GCS is enabled, and it is *enforced*.  If any binary is not
>>                     GCS-marked, or if kernel does not support, loading fails.
>>                     dlopen is always enforces GCS.
>>
>> It means that we can eventually set aarch64_gcs=1 as default in a future release,
>> and aarch64_gcs=2 might be an option for a hardened distro (or even in some future
>> release).
>>
> 
> So, right now what we have is 4 outcomes:
> 
> If aarch64_gcs is 0
> 
>  a) policy does not matter and GCS is disabled
> 
> If tunable is set, then
> 
>  b) policy 0: enable GCS regardless of marking
>  c) policy 1: run application correctly, with or without GCS depending on marking (see note below)
>  d) policy 2: run application only if it supports GCS (all binaries are marked), otherwise abort
>  
> Note: dlopen of unmarked binary is always an error when we care about markings (policies 1 and 2)
> 
> Policy 1 can switch off GCS but never via dlopen.

I still think we can just make it only one tunable, since with aarch64_gcs=0
policy does not matter (so it can be policy=0).

Also, policy=0 seems a debugging/enablement feature and once the program is
certified to work, you can just run it with policy 2.  The policy 1 is still
not really useful IMHO, since it means that enablement with policy=0 did not
catch potential issues with GCS (like a configuration where a dlopen is
not triggered).

> 
> I hope this helps,
> Yury
>