[2/2] jmpbuf: Add paddings for target specific usage
Commit Message
On Tue, Nov 7, 2017 at 10:05 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/07/2017 11:39 PM, H.J. Lu wrote:
>>
>> +typedef struct
>> + {
>> + __sigset_t __saved_mask;
>> + } __jmpbuf_target_t;
>
>
> “Target” in the sense of “target architecture”, not “jump target”? Maybe
> it's better to pick a different name.
I checked it to __jmpbuf_arch_t.
> Should <bits/setjmp3.h> be a header under bits/types/ instead?
>
I renamed it to bits/types/__jmpbuf_arch_t.h.
Here is the updated patch. Tested with build-many-glibcs.py.
Are there any comments or objections?
Thanks.
Comments
On 11/08/2017 07:27 PM, H.J. Lu wrote:
> +/* The biggest signal number + 1 */
> +#define _JUMP_BUF_SIGSET_NSIG 257
> +/* Number of longs to hold all signals. */
> +#define _JUMP_BUF_SIGSET_NWORDS \
> + ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
Where does 257 come from? 65 or 129 I would understand considering the
kernel sources, but 257 is odd.
I think it would be clearer to hard-code the array sizes and explain why
the values where chosen in that way.
We also need a test that setprocmask does not read from the previously
unused part. I can move the existing next_to_fault bits to support/ if
that would help.
Thanks,
Florian
On Mon, Nov 13, 2017 at 5:09 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/08/2017 07:27 PM, H.J. Lu wrote:
>>
>> +/* The biggest signal number + 1 */
>> +#define _JUMP_BUF_SIGSET_NSIG 257
>> +/* Number of longs to hold all signals. */
>> +#define _JUMP_BUF_SIGSET_NWORDS \
>> + ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
>
>
> Where does 257 come from? 65 or 129 I would understand considering the
> kernel sources, but 257 is odd.
I picked 257 to leave some room
> I think it would be clearer to hard-code the array sizes and explain why the
> values where chosen in that way.
>
> We also need a test that setprocmask does not read from the previously
Did you mean "sigprocmask"?
> unused part. I can move the existing next_to_fault bits to support/ if that
> would help.
Yes, please.
Thanks.
On 11/13/2017 03:05 PM, H.J. Lu wrote:
> On Mon, Nov 13, 2017 at 5:09 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 11/08/2017 07:27 PM, H.J. Lu wrote:
>>>
>>> +/* The biggest signal number + 1 */
>>> +#define _JUMP_BUF_SIGSET_NSIG 257
>>> +/* Number of longs to hold all signals. */
>>> +#define _JUMP_BUF_SIGSET_NWORDS \
>>> + ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
>>
>>
>> Where does 257 come from? 65 or 129 I would understand considering the
>> kernel sources, but 257 is odd.
Oh. I'm not sure if we should put this into installed header.
Maybe we can use a different approach? Something similar to the pthread
types? Or just not change the external type at all and just ad some
internal space reuse mechanism?
We had problems with people poking at supposedly invisible jmpbuf
contents in the past, and I'm worried that adding even __ members will
encourage that.
>> I think it would be clearer to hard-code the array sizes and explain why the
>> values where chosen in that way.
>>
>> We also need a test that setprocmask does not read from the previously
>
> Did you mean "sigprocmask"?
Right.
>> unused part. I can move the existing next_to_fault bits to support/ if that
>> would help.
>
> Yes, please.
Okay, I'll move it to support/ soon.
Thanks,
Florian
On Mon, Nov 13, 2017 at 8:34 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 11/13/2017 03:05 PM, H.J. Lu wrote:
>>
>> On Mon, Nov 13, 2017 at 5:09 AM, Florian Weimer <fweimer@redhat.com>
>> wrote:
>>>
>>> On 11/08/2017 07:27 PM, H.J. Lu wrote:
>>>>
>>>>
>>>> +/* The biggest signal number + 1 */
>>>> +#define _JUMP_BUF_SIGSET_NSIG 257
>>>> +/* Number of longs to hold all signals. */
>>>> +#define _JUMP_BUF_SIGSET_NWORDS \
>>>> + ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
>>>
>>>
>>>
>>> Where does 257 come from? 65 or 129 I would understand considering the
>>> kernel sources, but 257 is odd.
>
>
> Oh. I'm not sure if we should put this into installed header.
>
> Maybe we can use a different approach? Something similar to the pthread
> types? Or just not change the external type at all and just ad some
> internal space reuse mechanism?
I will see what I can do.
> We had problems with people poking at supposedly invisible jmpbuf contents
> in the past, and I'm worried that adding even __ members will encourage
> that.
>
>>> I think it would be clearer to hard-code the array sizes and explain why
>>> the
>>> values where chosen in that way.
>>>
>>> We also need a test that setprocmask does not read from the previously
>>
>>
>> Did you mean "sigprocmask"?
>
>
> Right.
>
>>> unused part. I can move the existing next_to_fault bits to support/ if
>>> that
>>> would help.
>>
>>
>> Yes, please.
>
>
> Okay, I'll move it to support/ soon.
>
Here is what the test will look like
static int
do_test (void)
{
struct __jmp_buf_tag *sj = memory from next_to_fault
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"SJ" will point to the end of page in such a way that sigprocmask
will fail if it reads beyond the actual __saved_mask.
sigset_t m;
sigemptyset (&m);
sigprocmask (SIG_SETMASK, &m, NULL);
if (sigsetjmp (sj, 0) == 0)
{
sigaddset (&m, SIGUSR1);
sigprocmask (SIG_SETMASK, &m, NULL);
siglongjmp (sj, 1);
return EXIT_FAILURE;
}
sigprocmask (SIG_SETMASK, NULL, &m);
return sigismember (&m, SIGUSR1) ? EXIT_SUCCESS : EXIT_FAILURE;
}
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
Thanks.
From 21223ad34a68a2053a78efcee4856733eb4b46e0 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 10 Jun 2017 17:23:06 -0700
Subject: [PATCH 2/2] jmpbuf: Add paddings for architecture specific usage
To support Shadow Stack (SHSTK) in Intel Control-flow Enforcement
Technology (CET) in setjmp/longjmp, we need to save shadow stack
pointer in jmp_buf. The __saved_mask field in jmp_buf has type
of __sigset_t. On Linux, __sigset_t is defined as
#define _SIGSET_NWORDS (1024 / (8 * sizeof (unsigned long int)))
typedef struct
{
unsigned long int __val[_SIGSET_NWORDS];
} __sigset_t;
which is much bigger than expected by the __sigprocmask system call,
which has
typedef struct {
unsigned long sig[_NSIG_WORDS];
} sigset_t;
We can shrink __sigset_t used by __saved_mask in jmp_buf to add paddings
for architecture specific usage. As long as the new __sigset_t is not
smaller than sigset_t expected by the __sigprocmask system call, it should
work correctly. This patch defines __jmp_buf_sigset_t for __saved_mask in
jmp_buf on Linux and verifies __jmp_buf_sigset_t has the suitable size
for the __sigprocmask system call.
Tested with build-many-glibcs.py.
* bits/types/__jmpbuf_arch_t.h: New file.
* sysdeps/unix/sysv/linux/bits/types/__jmpbuf_arch_t.h: Likewise.
* include/setjmp.h [!_ISOMAC]: Include <signal.h>.
[_NSIG] (_SIGPROCMASK_NSIG_WORDS): New.
[_NSIG] (__sigprocmask_sigset_t): Likewise.
[_NSIG] (do_test): Add _Static_assert for size of __saved_mask
in jmp_buf >= sizeof __sigprocmask_sigset_t.
* setjmp/Makefile (headers): Add bits/types/__jmpbuf_arch_t.h.
* setjmp/longjmp.c (__libc_siglongjmp): Cast &env[0].__saved_mask
to "sigset_t *".
* setjmp/sigjmp.c (__sigjmp_save): Likewise.
* sysdeps/powerpc/longjmp.c (__vmx__libc_siglongjmp): Likewise.
* sysdeps/powerpc/novmx-longjmp.c (__novmx__libc_siglongjmp):
Likewise.
* sysdeps/powerpc/novmx-sigjmp.c (__novmx__sigjmp_save): Likewise.
* sysdeps/powerpc/sigjmp.c (__vmx__sigjmp_save): Likewise.
* sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c
(__libc_unwind_longjmp): Likewise.
* setjmp/setjmp.h: Include <bits/types/__jmpbuf_arch_t.h> instead
of <bits/types/__sigset_t.h>.
(__jmp_buf_tag): Change __saved_mask to __arch.
---
bits/types/__jmpbuf_arch_t.h | 31 ++++++++++++++
include/setjmp.h | 15 +++++++
setjmp/Makefile | 3 +-
setjmp/longjmp.c | 3 +-
setjmp/setjmp.h | 4 +-
setjmp/sigjmp.c | 2 +-
sysdeps/powerpc/longjmp.c | 3 +-
sysdeps/powerpc/novmx-longjmp.c | 3 +-
sysdeps/powerpc/novmx-sigjmp.c | 2 +-
sysdeps/powerpc/sigjmp.c | 2 +-
.../unix/sysv/linux/bits/types/__jmpbuf_arch_t.h | 48 ++++++++++++++++++++++
sysdeps/unix/sysv/linux/ia64/unwind_longjmp.c | 3 +-
12 files changed, 109 insertions(+), 10 deletions(-)
create mode 100644 bits/types/__jmpbuf_arch_t.h
create mode 100644 sysdeps/unix/sysv/linux/bits/types/__jmpbuf_arch_t.h
new file mode 100644
@@ -0,0 +1,31 @@
+/* Generic __jmpbuf_arch_t defition.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SETJMP_H
+# error "Never include <bits/types/__jmpbuf_arch_t.h directly; use <setjmp.h> instead."
+#endif
+
+#include <bits/types/__sigset_t.h>
+
+typedef struct
+ {
+ __sigset_t __saved_mask;
+ } __jmpbuf_arch_t;
+
+/* Saved signal mask. */
+#define __saved_mask __arch.__saved_mask
@@ -33,6 +33,7 @@ extern __typeof (__sigsetjmp) __sigsetjmp attribute_hidden;
# endif
/* Check jmp_buf sizes, alignments and offsets. */
+# include <signal.h>
# include <stddef.h>
# include <jmp_buf-macros.h>
@@ -65,6 +66,20 @@ TEST_OFFSET (struct __jmp_buf_tag, __mask_was_saved,
MASK_WAS_SAVED_OFFSET);
TEST_OFFSET (struct __jmp_buf_tag, __saved_mask,
SAVED_MASK_OFFSET);
+
+# ifdef _NSIG
+# define _SIGPROCMASK_NSIG_WORDS (_NSIG / (8 * sizeof (unsigned long int)))
+
+typedef struct
+ {
+ unsigned long int __val[_SIGPROCMASK_NSIG_WORDS];
+ } __sigprocmask_sigset_t;
+
+extern jmp_buf ___buf;
+extern __typeof (___buf[0].__saved_mask) ___saved_mask;
+_Static_assert (sizeof (___saved_mask) >= sizeof (__sigprocmask_sigset_t),
+ "size of ___saved_mask < size of __sigprocmask_sigset_t");
+# endif
#endif
#endif
@@ -22,7 +22,8 @@ subdir := setjmp
include ../Makeconfig
-headers := setjmp.h bits/setjmp.h bits/setjmp2.h
+headers := setjmp.h bits/setjmp.h bits/setjmp2.h \
+ bits/types/__jmpbuf_arch_t.h
routines := setjmp sigjmp bsd-setjmp bsd-_setjmp \
longjmp __longjmp jmp-unwind
@@ -31,7 +31,8 @@ __libc_siglongjmp (sigjmp_buf env, int val)
if (env[0].__mask_was_saved)
/* Restore the saved signal mask. */
- (void) __sigprocmask (SIG_SETMASK, &env[0].__saved_mask,
+ (void) __sigprocmask (SIG_SETMASK,
+ (sigset_t *) &env[0].__saved_mask,
(sigset_t *) NULL);
/* Call the machine-dependent function to restore machine state. */
@@ -27,7 +27,7 @@
__BEGIN_DECLS
#include <bits/setjmp.h> /* Get `__jmp_buf'. */
-#include <bits/types/__sigset_t.h>
+#include <bits/types/__jmpbuf_arch_t.h>
/* Calling environment, plus possibly a saved signal mask. */
struct __jmp_buf_tag
@@ -38,7 +38,7 @@ struct __jmp_buf_tag
or add others before it. */
__jmp_buf __jmpbuf; /* Calling environment. */
int __mask_was_saved; /* Saved the signal mask? */
- __sigset_t __saved_mask; /* Saved signal mask. */
+ __jmpbuf_arch_t __arch; /* Architecture specific data. */
};
@@ -28,7 +28,7 @@ __sigjmp_save (sigjmp_buf env, int savemask)
{
env[0].__mask_was_saved = (savemask &&
__sigprocmask (SIG_BLOCK, (sigset_t *) NULL,
- &env[0].__saved_mask) == 0);
+ (sigset_t *) &env[0].__saved_mask) == 0);
return 0;
}
@@ -39,7 +39,8 @@ __vmx__libc_siglongjmp (sigjmp_buf env, int val)
if (env[0].__mask_was_saved)
/* Restore the saved signal mask. */
- (void) __sigprocmask (SIG_SETMASK, &env[0].__saved_mask,
+ (void) __sigprocmask (SIG_SETMASK,
+ (sigset_t *) &env[0].__saved_mask,
(sigset_t *) NULL);
/* Call the machine-dependent function to restore machine state. */
@@ -37,7 +37,8 @@ __novmx__libc_siglongjmp (__novmx__sigjmp_buf env, int val)
if (env[0].__mask_was_saved)
/* Restore the saved signal mask. */
- (void) __sigprocmask (SIG_SETMASK, &env[0].__saved_mask,
+ (void) __sigprocmask (SIG_SETMASK,
+ (sigset_t *) &env[0].__saved_mask,
(sigset_t *) NULL);
/* Call the machine-dependent function to restore machine state. */
@@ -35,7 +35,7 @@ __novmx__sigjmp_save (__novmx__sigjmp_buf env, int savemask)
{
env[0].__mask_was_saved = (savemask &&
__sigprocmask (SIG_BLOCK, (sigset_t *) NULL,
- &env[0].__saved_mask) == 0);
+ (sigset_t *) &env[0].__saved_mask) == 0);
return 0;
}
@@ -31,7 +31,7 @@ __vmx__sigjmp_save (sigjmp_buf env, int savemask)
{
env[0].__mask_was_saved = (savemask &&
__sigprocmask (SIG_BLOCK, (sigset_t *) NULL,
- &env[0].__saved_mask) == 0);
+ (sigset_t *) &env[0].__saved_mask) == 0);
return 0;
}
new file mode 100644
@@ -0,0 +1,48 @@
+/* __jmpbuf_arch_t defition for Linux.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SETJMP_H
+# error "Never include <bits/types/__jmpbuf_arch_t.h> directly; use <setjmp.h> instead."
+#endif
+
+#include <bits/types/__sigset_t.h>
+
+/* The biggest signal number + 1 */
+#define _JUMP_BUF_SIGSET_NSIG 257
+/* Number of longs to hold all signals. */
+#define _JUMP_BUF_SIGSET_NWORDS \
+ ((_JUMP_BUF_SIGSET_NSIG - 1 + 7) / (8 * sizeof (unsigned long int)))
+
+typedef struct
+ {
+ unsigned long int __val[_JUMP_BUF_SIGSET_NWORDS];
+ } __jmp_buf_sigset_t;
+
+typedef union
+ {
+ __sigset_t __saved_mask_compat;
+ struct
+ {
+ __jmp_buf_sigset_t __saved_mask;
+ /* Paddings for target specific usage. */
+ unsigned long int __padding[12];
+ } __saved;
+ } __jmpbuf_arch_t;
+
+/* Saved signal mask. */
+#define __saved_mask __arch.__saved.__saved_mask
@@ -34,7 +34,8 @@ __libc_unwind_longjmp (sigjmp_buf env, int val)
if (env[0].__mask_was_saved)
/* Restore the saved signal mask. */
- (void) __sigprocmask (SIG_SETMASK, &env[0].__saved_mask,
+ (void) __sigprocmask (SIG_SETMASK,
+ (sigset_t *) &env[0].__saved_mask,
(sigset_t *) NULL);
/* Call the machine-dependent function to restore machine state. */
--
2.13.6