linux-hardening - Re: [PATCH] [PATCH,v5,1/1,AARCH64][PR102768] aarch64: Add compiler support for Shadow Call Stack

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: <94c7f7e0-055e-d235-4595-7e335b4ac273@linux.alibaba.com>
Date:   Wed, 16 Feb 2022 08:04:02 -0800
From:   Dan Li <ashimida@...ux.alibaba.com>
To:     gcc-patches@....gnu.org, richard.earnshaw@....com,
        marcus.shawcroft@....com, kyrylo.tkachov@....com, hp@....gnu.org,
        ndesaulniers@...gle.com, nsz@....gnu.org, pageexec@...il.com,
        qinzhao@....gnu.org, linux-hardening@...r.kernel.org,
        richard.sandiford@....com
Subject: Re: [PATCH] [PATCH,v5,1/1,AARCH64][PR102768] aarch64: Add compiler
 support for Shadow Call Stack



On 2/15/22 10:02, Richard Sandiford wrote:
> Dan Li <ashimida@...ux.alibaba.com> writes:
>> Shadow Call Stack can be used to protect the return address of a
>> function at runtime, and clang already supports this feature[1].
> 
> Looks good, thanks.  However, when I bootstrap it on
> aarch64-linux-gnu I get:
> 
> .../gcc/ubsan.cc: In function ‘bool ubsan_expand_null_ifn(gimple_stmt_iterator*)’:
> .../gcc/ubsan.cc:835:50: error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
>    835 |         = (flag_sanitize_recover & ((check_align ? SANITIZE_ALIGNMENT : 0)
>        |                                      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
> .../gcc/ubsan.cc:836:51: error: enumerated and non-enumerated type in conditional expression [-Werror=extra]
>    836 |                                     | (check_null ? SANITIZE_NULL : 0)))
>        |                                        ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> 
> I think this is because you're taking the last available bit
> of the enum :-)
> 
> A hacky fix is to add "+ 0" to SANITIZE_ALIGNMENT and SANITIZE_NULL
> in the code quoted above (i.e. the code in the error messages).
> That seems slightly more robust than a cast to unsigned int (say),
> since "+ 0" will work even if the values become 64-bit quantities
> in future.
> 
> Richard
> 

Ah, apologize for my mistake! I specified --disable-werror in
./configure from the beginning, I didn't see this problem before.

As you said, I use the following patch:

diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc
index 5641d3cc3be..a858994c841 100644
--- a/gcc/ubsan.cc
+++ b/gcc/ubsan.cc
@@ -832,8 +832,8 @@ ubsan_expand_null_ifn (gimple_stmt_iterator *gsip)
    else
      {
        enum built_in_function bcode
-       = (flag_sanitize_recover & ((check_align ? SANITIZE_ALIGNMENT : 0)
-                                   | (check_null ? SANITIZE_NULL : 0)))
+       = (flag_sanitize_recover & ((check_align ? SANITIZE_ALIGNMENT + 0 : 0)
+                                   | (check_null ? SANITIZE_NULL + 0 : 0)))
           ? BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH_V1
           : BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH_V1_ABORT;
        tree fn = builtin_decl_implicit (bcode);


And tested fine in native compiling for x86_64 , I will change it in the
next version.

BTW:
The platform I'm using is x86-64, so I'm trying to find a way to
reproduce this issue when cross-compiling with aarch64, which I
haven't found so far, the issue only seems to happen with native
compilation.

But most of the code changes are for the aarch64 platform,
is it enough for me to do the following tests before submitting
the patch?
1) A full compile of gcc under x86_64 platform
(make; make install; make bootstrap;)
2) Test all testsuites in aarch64 cross-compile environment
(make -k check)


Thanks,
Dan