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

[Adhemerval Zanella Netto]

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:

 40 static void
 41 check_gcs (struct link_map *l, const char *program)
 42 {
 43 #ifdef SHARED
 44   /* Ignore GCS marking on ld.so: its properties are not processed.  */
 45   if (is_rtld_link_map (l->l_real))
 46     return;
 47 #endif
 48   bool for_dlopen = program == NULL;
 49   if (!l->l_mach.gcs)
 50     {
 51       if (GLRO(dl_aarch64_gcs_policy) == 2 || for_dlopen)
 52         fail (l, program);
 53       if (GLRO(dl_aarch64_gcs_policy) == 1)
 54         GL(dl_aarch64_gcs) = 0;
 55       else
 56         unsupported ();
 57

For dl_aarch64_gcs_policy=1, it silent changes the dl_aarch64_gcs to 0 which
disable future GCS checks.

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.  And still think this semantic is far from ideal.

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

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

> 
>>
>> As I said, we recently removed a similar support for executable stacks
>> (commit 0ca8785a28) because this is silent change of process semantic and
>> it also used as way to enable a RCE on a CVE [1].
>>
>> So I don't think we should explict support options to allow process to move
>> from a hardened to a lower hardened security perpective, specially in a
>> silent manner, and even though a explicit option (the tunable, where use
>> does need to opt-in).
>>
>> It would be good to have some more input for other maintainers.  How x86 does
>> in this regard (I haven't check it).
>>
>> [1] https://www.qualys.com/2023/07/19/cve-2023-38408/rce-openssh-forwarded-ssh-agent.txt
> 
> 
> Thanks,
> Yury
>