[Bug middle-end/58245] New: -fstack-protector[-all] does not protect functions that call noreturn functions

[Bug middle-end/58245] New: -fstack-protector[-all] does not protect functions that call noreturn functions

[bugdal at aerifal dot cx]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

            Bug ID: 58245
           Summary: -fstack-protector[-all] does not protect functions
                    that call noreturn functions
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bugdal at aerifal dot cx

This issue is almost identical to bug #23221, but affects functions whose
executions end with a call to a noreturn function rather than a tail call. The
simplest example is:

#include <stdlib.h>
int main()
{
    exit(0);
}

When compiled with -fstack-protector-all, the function prologue will read and
store the canary, but no check will be made before passing execution to exit.

This is actually a major practical problem for some users of musl libc, because
code like the above appears in many configure scripts, and musl libc uses weak
symbols so as to avoid initializing stack-protector (and thereby avoid
initializing the TLS register) if there is no reference to __stack_chk_fail.
Due to this issue, the above code generates thread-pointer-relative (e.g.
%fs-based on x86_64) accesses to read the canary, but no reference to
__stack_chk_fail, and then crashes when run, leading to spurious configure
failures. For the time being, I have informed users who wish to use
-fstack-protector-all that they can add -fno-builtin-exit -D__noreturn__= to
their CFLAGS, but this is an ugly workaround.

It should be noted that this issue happens even at -O0. I think using noreturn
for dead code removal at -O0 is highly undesirable; for instance, it would
preclude proper debugging of issues caused by a function erroneously being
marked noreturn and actually returning. However that matter probably deserves
its own bug report...

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[bugdal at aerifal dot cx]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #1 from Rich Felker <bugdal at aerifal dot cx> ---
One more thing: I would be happy with either of two solutions, either:

(1) Checking the canary before calling a noreturn function, just like
performing a check before a tail-call, or

(2) Eliminating the dead-code-removal of the function epilogue at -O0, and for
non-zero -O levels, adding an optimization to remove the canary loading from
the prologue if no epilogue to check the canary is to be generated.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The best solution: Don't use the same triplet as the GNU (glibc) one.  Have
musl have its own triplet.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[bugdal at aerifal dot cx]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #3 from Rich Felker <bugdal at aerifal dot cx> ---
We already do that; the patch is in the musl-cross repo here:

https://bitbucket.org/GregorR/musl-cross or
https://github.com/GregorR/musl-cross

However, we want the stack-protector behavior for GCC with musl to be the same
as with glibc, using the TLS canary and __stack_chk_fail function in libc
rather than a separate libssp. In all real-world, nontrivial code, everything
works fine. The only failure of empty programs like the above which just call
exit, which, when combined with -fstack-protector-all, cause failure.

In any case, the failure of configure scripts with musl is just one symptom of
the problem: useless loads of the canary without a corresponding check of the
canary. From a security standpoint, I feel like checking the canary before
calling a function that won't return would be the best possible behavior, so
that every function gets a check. However, if doing this isn't deemed
worthwhile, I think the canary load, which is dead code without a subsequent
check, should be optimized out.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[rose.garcia-eggl2fk at yopmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

Rose Garcia <rose.garcia-eggl2fk at yopmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rose.garcia-eggl2fk@yopmail
                   |                            |.com

--- Comment #4 from Rose Garcia <rose.garcia-eggl2fk at yopmail dot com> ---
the place to get only the GCC patches for musl is
https://github.com/GregorR/musl-gcc-patches though.


btw, some debugging has shown that
stack_protect_prologue() and
stack_protect_epilogue() are both called, so it looks as if it's a later pass
that decides to eliminate the "dead" code of the epilogue, but not the one of
the prologue.

any hints where in the code this could happen is appreciated.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[timo.teras at iki dot fi]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #5 from Timo Teräs <timo.teras at iki dot fi> ---
I have the same issue and confirm this issue. Any ideas how to fix it properly?
>From gcc-bugs-return-430903-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Tue Oct 01 14:49:43 2013
Return-Path: <gcc-bugs-return-430903-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 32184 invoked by alias); 1 Oct 2013 14:49:43 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 32146 invoked by uid 55); 1 Oct 2013 14:49:40 -0000
From: "jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug libgomp/57298] GOMP_CPU_AFFINITY will not work when system has >1024 cores
Date: Tue, 01 Oct 2013 14:49:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: libgomp
X-Bugzilla-Version: 4.8.1
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: jakub at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-57298-4-JpQ1gaxqMo@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-57298-4@http.gcc.gnu.org/bugzilla/>
References: <bug-57298-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-10/txt/msg00047.txt.bz2
Content-length: 1950

http://gcc.gnu.org/bugzilla/show_bug.cgi?idW298

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Tue Oct  1 14:49:36 2013
New Revision: 203064

URL: http://gcc.gnu.org/viewcvs?rev 3064&root=gcc&view=rev
Log:
    PR libgomp/57298
    * config/linux/proc.c (gomp_cpuset_size, gomp_cpusetp): New variables.
    (gomp_cpuset_popcount): Use CPU_COUNT_S if available, or CPU_COUNT if
    gomp_cpuset_size is sizeof (cpu_set_t).  Use gomp_cpuset_size instead
    of sizeof (cpu_set_t) to determine number of iterations.
    (gomp_init_num_threads): Initialize gomp_cpuset_size and gomp_cpusetp
    here, use gomp_cpusetp instead of &cpuset and pass gomp_cpuset_size
    instead of sizeof (cpu_set_t) to pthread_getaffinity_np.
    (get_num_procs): Don't call pthread_getaffinity_np if gomp_cpusetp
    is NULL.  Use gomp_cpusetp instead of &cpuset and pass gomp_cpuset_size
    instead of sizeof (cpu_set_t) to pthread_getaffinity_np.
    * config/linux/proc.h (gomp_cpuset_popcount): Add attribute_hidden.
    (gomp_cpuset_size, gomp_cpusetp): Declare.
    * config/linux/affinity.c (CPU_ISSET_S, CPU_ZERO_S, CPU_SET_S): Define
    if CPU_ALLOC_SIZE isn't defined.
    (gomp_init_affinity): Don't call pthread_getaffinity_np here, instead
    use gomp_cpusetp computed by gomp_init_num_threads.  Use CPU_*_S
    variants of macros with gomp_cpuset_size as set size, for cpusetnew
    use alloca for it if CPU_ALLOC_SIZE is defined, otherwise local
    fixed size variable.
    (gomp_init_thread_affinity): Use CPU_*_S variants of macros with
    gomp_cpuset_size as set size, for cpuset use alloca for it if
    CPU_ALLOC_SIZE is defined, otherwise local fixed size variable.

Modified:
    branches/gomp-4_0-branch/libgomp/ChangeLog.gomp
    branches/gomp-4_0-branch/libgomp/config/linux/affinity.c
    branches/gomp-4_0-branch/libgomp/config/linux/proc.c
    branches/gomp-4_0-branch/libgomp/config/linux/proc.h

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I don't see how this can be considered a GCC bug.
GCC doesn't check the canary before any other functions in the body of the
function, the reason for checking the stack canary is to prevent using
overwritten return addresses on the stack, but noreturn function doesn't
return, so it is irrelevant if the stack has been overwritten (except for what
would matter inside of the function itself, but we don't check it for any other
function either in the middle of functions).

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[bugdal at aerifal dot cx]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #7 from Rich Felker <bugdal at aerifal dot cx> ---
I claim it's a bug in that GCC should _either_ check the canary at some point,
or eliminate the code that's loading the canary to begin with since it will
never be checked.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[basile at opensource dot dyc.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

Anthony G. Basile <basile at opensource dot dyc.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |basile at opensource dot dyc.edu

--- Comment #8 from Anthony G. Basile <basile at opensource dot dyc.edu> ---
Gentoo's hardened toolchain is stumbling on this bug and its a blocker for us
to get musl + gentoo working nicely.  Has there been any movement on it?

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[jackie.rosen at hushmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #9 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 106288 has been marked as a duplicate of this bug. ***

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #12 from H.J. Lu <hjl.tools at gmail dot com> ---
Created attachment 53294
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53294&action=edit
A patch

Something like this?

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-07-13
            Version|unknown                     |10.3.1

[Bug middle-end/58245] -fstack-protector[-all] does not protect functions that call noreturn functions

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58245

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by H.J. Lu <hjl@gcc.gnu.org>:

https://gcc.gnu.org/g:a25982ada523689c8745d7fb4b1b93c8f5dab2e7

commit r13-2909-ga25982ada523689c8745d7fb4b1b93c8f5dab2e7
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jul 14 08:23:38 2022 -0700

    stack-protector: Check stack canary before throwing exception

    Check stack canary before throwing exception to avoid stack corruption.

    gcc/

            PR middle-end/58245
            * calls.cc: Include "tree-eh.h".
            (expand_call): Check stack canary before throwing exception.

    gcc/testsuite/

            PR middle-end/58245
            * g++.dg/fstack-protector-strong.C: Adjusted.
            * g++.dg/pr58245-1.C: New test.