Bug 24110 - SS_DISABLE never set in stack_t value returned by sigaltstack
Summary: SS_DISABLE never set in stack_t value returned by sigaltstack
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: hurd (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: 2.29
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-20 16:37 UTC by Bruno Haible
Modified: 2019-02-11 11:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Test case (1.00 KB, text/x-csrc)
2019-01-20 16:37 UTC, Bruno Haible
Details
Output on Hurd 0.9 (with glibc 2.24) (152 bytes, text/plain)
2019-01-20 16:39 UTC, Bruno Haible
Details
Output on Linux (with glibc) (162 bytes, text/plain)
2019-01-20 16:40 UTC, Bruno Haible
Details
proposed fix (260 bytes, patch)
2019-01-20 16:48 UTC, Samuel Thibault
Details | Diff
Possible fix (untested) (248 bytes, patch)
2019-01-20 16:57 UTC, Bruno Haible
Details | Diff
Possible fix (untested) (253 bytes, patch)
2019-01-20 16:58 UTC, Bruno Haible
Details | Diff
Another test program (225 bytes, text/plain)
2019-01-20 17:24 UTC, Bruno Haible
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Haible 2019-01-20 16:37:55 UTC
Created attachment 11550 [details]
Test case

When sigaltstack() is used to query the current state of the alternate stack
(a) immediately after entering main(),
(b) in a freshly created thread,
on GNU Hurd 0.9 it returns a 'stack_t' value with SS_DISABLE not set in ss_flags, and ss_sp and ss_size both 0.

According to POSIX https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigaltstack.html
"The ss_flags member specifies the new stack state. If it is set to SS_DISABLE, the stack is disabled and ss_sp and ss_size are ignored. Otherwise, the stack shall be enabled, and the ss_sp and ss_size members specify the new address and size of the stack."

So the sigaltstack() is reporting that an alternate stack is enabled but with a stack of size 0 at address 0 - which makes no sense.

On all other platforms I tested (Linux, macOS, FreeBSD, NetBSD, OpenBSD, AIX, HP-UX, Solaris, Haiku, Cygwin), the 'stack_t' value has SS_DISABLE set in ss_flags in these situations.

How to reproduce:
$ gcc -Wall foo.c -lpthread
$ ./a.out
Comment 1 Bruno Haible 2019-01-20 16:39:05 UTC
Created attachment 11551 [details]
Output on Hurd 0.9 (with glibc 2.24)
Comment 2 Bruno Haible 2019-01-20 16:40:01 UTC
Created attachment 11552 [details]
Output on Linux (with glibc)
Comment 3 Samuel Thibault 2019-01-20 16:48:19 UTC
Created attachment 11553 [details]
proposed fix

So AIUI the issue is just that SS_DISABLE is missing in the flags?

Could somebody test the attached patch?
Comment 4 Bruno Haible 2019-01-20 16:57:27 UTC
Created attachment 11554 [details]
Possible fix (untested)
Comment 5 Bruno Haible 2019-01-20 16:58:14 UTC
Created attachment 11555 [details]
Possible fix (untested)

Attaching two possible fixes. Both untested.
Comment 6 Bruno Haible 2019-01-20 17:00:24 UTC
(In reply to Samuel Thibault from comment #3)
> So AIUI the issue is just that SS_DISABLE is missing in the flags?

Yes.
Comment 7 Bruno Haible 2019-01-20 17:23:42 UTC
This is not the only effect, after all.

When a signal has a handler set with SA_ONSTACK, and the thread is freshly initialized and then the signal is raised, the program will crash.

How to reproduce: Run the attached program use-initial-alternate-stack.c. It succeeds on Linux, but crashes (with 'Illegal instruction') on Hurd.

How come? Look at glibc/sysdeps/mach/hurd/i386/trampoline.c line 80

sigsp = ss->sigaltstack.ss_sp + ss->sigaltstack.ss_size;

Here a consequence of the missing SS_DISABLE flag upon initialization is that this code will attempt to switch to the alternate signal stack, placing a stack frame just below address 0x00000000.
Comment 8 Bruno Haible 2019-01-20 17:24:47 UTC
Created attachment 11556 [details]
Another test program
Comment 9 cvs-commit@gcc.gnu.org 2019-01-24 19:17:10 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  b96e22d080688867d24a9015fd50c199144d0e47 (commit)
      from  22ff602427177efbbe36939901c3c5239767b960 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b96e22d080688867d24a9015fd50c199144d0e47

commit b96e22d080688867d24a9015fd50c199144d0e47
Author: Samuel Thibault <samuel.thibault@ens-lyon.org>
Date:   Thu Jan 24 20:15:01 2019 +0100

    hurd: Fix initial sigaltstack state
    
    Previous commit fixed
    [BZ #24110]

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Comment 10 joseph@codesourcery.com 2019-01-24 20:25:51 UTC
If the bug is fixed, please mark it as such with target milestone set so 
the list of fixed bugs in NEWS can be generated properly.
Comment 11 Samuel Thibault 2019-01-24 22:42:43 UTC
Ok, thanks!