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

[Yury Khrustalev]

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).

> 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.

> 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".

>                     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 hope this helps,
Yury