This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] jmpbuf: Add paddings for target specific usage


On Mon, Nov 13, 2017 at 11:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Nov 13, 2017 at 8:44 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> 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 the updated patch to add <setjmpP.h>.  I added a test:
>
>   struct support_next_to_fault jmpbuf
>     = support_next_to_fault_allocate (SAVED_MASK_OFFSET + (_NSIG / 8));
>   struct __jmp_buf_tag *sj = (struct __jmp_buf_tag *) jmpbuf.buffer;
>
>   errno = 0;
>   if (sigsetjmp (sj, 1) == 0)
>     {
>       siglongjmp (sj, 1);
>       return EXIT_FAILURE;
>     }
>   if (errno != 0)
>     {
>       printf ("sigsetjmp: %s\n", strerror (errno));
>       return EXIT_FAILURE;
>     }
>
> to verify that __sigprocmask won't read beyond _NSIG / 8.
>
> Does it look OK?
>

This is the updated patch.  Tested with build-many-glibcs.py.

Any other comments?

Thanks.


-- 
H.J.
From 2bad2646b07903c0e9ba5b63656c5d2bbae4f6cf 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] 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 adds an internal header file, <setjmpP.h>,
to define __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.

	* debug/longjmp_chk.c: Include <setjmpP.h> instead of
	<setjmp.h>.
	* sysdeps/unix/sysv/linux/s390/longjmp_chk.c: Likewise.
	* include/setjmpP.h: New file.
	* setjmp/setjmpP.h: Likewise.
	* sysdeps/generic/__jmpbuf_arch_t.h: Likewise.
	* sysdeps/generic/__saved_mask.h: Likewise.
	* sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h: Likewise.
	* sysdeps/unix/sysv/linux/__saved_mask.h: Likewise.
	* setjmp/tst-sigsetjmp2.c: 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 (tests): Add tst-sigsetjmp2.
	* setjmp/longjmp.c: Include <setjmpP.h> instead of <setjmp.h>.
	(__libc_siglongjmp): Cast &env[0].__saved_mask to "sigset_t *".
	* setjmp/sigjmp.c: Include <setjmpP.h> instead of <setjmp.h>.
	(__sigjmp_save): Cast &env[0].__saved_mask to "sigset_t *".
---
 debug/longjmp_chk.c                        |  2 +-
 include/setjmp.h                           | 15 +++++++++
 include/setjmpP.h                          |  1 +
 setjmp/Makefile                            |  2 +-
 setjmp/longjmp.c                           |  5 +--
 setjmp/setjmpP.h                           | 32 +++++++++++++++++++
 setjmp/sigjmp.c                            |  4 +--
 setjmp/tst-sigsetjmp2.c                    | 50 ++++++++++++++++++++++++++++++
 sysdeps/generic/__jmpbuf_arch_t.h          | 24 ++++++++++++++
 sysdeps/generic/__saved_mask.h             | 20 ++++++++++++
 sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h  | 43 +++++++++++++++++++++++++
 sysdeps/unix/sysv/linux/__saved_mask.h     | 20 ++++++++++++
 sysdeps/unix/sysv/linux/s390/longjmp_chk.c |  2 +-
 13 files changed, 213 insertions(+), 7 deletions(-)
 create mode 100644 include/setjmpP.h
 create mode 100644 setjmp/setjmpP.h
 create mode 100644 setjmp/tst-sigsetjmp2.c
 create mode 100644 sysdeps/generic/__jmpbuf_arch_t.h
 create mode 100644 sysdeps/generic/__saved_mask.h
 create mode 100644 sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h
 create mode 100644 sysdeps/unix/sysv/linux/__saved_mask.h

diff --git a/debug/longjmp_chk.c b/debug/longjmp_chk.c
index 1cea6c05ef..61be0da0cd 100644
--- a/debug/longjmp_chk.c
+++ b/debug/longjmp_chk.c
@@ -15,7 +15,7 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <setjmp.h>
+#include <setjmpP.h>
 
 #define __longjmp ____longjmp_chk
 #define __libc_siglongjmp __longjmp_chk
diff --git a/include/setjmp.h b/include/setjmp.h
index f1b19f5ceb..91d916212c 100644
--- a/include/setjmp.h
+++ b/include/setjmp.h
@@ -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
diff --git a/include/setjmpP.h b/include/setjmpP.h
new file mode 100644
index 0000000000..8aa6a25957
--- /dev/null
+++ b/include/setjmpP.h
@@ -0,0 +1 @@
+#include <setjmp/setjmpP.h>
diff --git a/setjmp/Makefile b/setjmp/Makefile
index ca80b8ea13..e0a1f4a30b 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -28,7 +28,7 @@ routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
 		   longjmp __longjmp jmp-unwind
 
 tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
-		   tst-sigsetjmp tst-setjmp-static
+		   tst-sigsetjmp tst-setjmp-static tst-sigsetjmp2
 tests-static	:= tst-setjmp-static
 
 
diff --git a/setjmp/longjmp.c b/setjmp/longjmp.c
index 2453c2c124..e144a874d7 100644
--- a/setjmp/longjmp.c
+++ b/setjmp/longjmp.c
@@ -16,7 +16,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <stddef.h>
-#include <setjmp.h>
+#include <setjmpP.h>
 #include <signal.h>
 
 
@@ -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.  */
diff --git a/setjmp/setjmpP.h b/setjmp/setjmpP.h
new file mode 100644
index 0000000000..e65d72b654
--- /dev/null
+++ b/setjmp/setjmpP.h
@@ -0,0 +1,32 @@
+/* Internal __sigset_t type used in struct __jmp_buf_tag.
+   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/>.  */
+
+/*
+ *	ISO C99 Standard: 7.13 Nonlocal jumps	<setjmp.h>
+ */
+
+#ifndef	_SETJMPP_H
+#define	_SETJMPP_H	1
+
+#include <__jmpbuf_arch_t.h>
+#undef __sigset_t
+#define __sigset_t __jmpbuf_arch_t
+#include <setjmp.h>
+#include <__saved_mask.h>
+
+#endif /* setjmpP.h  */
diff --git a/setjmp/sigjmp.c b/setjmp/sigjmp.c
index 30839ae819..32b727fae8 100644
--- a/setjmp/sigjmp.c
+++ b/setjmp/sigjmp.c
@@ -16,7 +16,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <stddef.h>
-#include <setjmp.h>
+#include <setjmpP.h>
 #include <signal.h>
 
 /* This function is called by the `sigsetjmp' macro
@@ -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;
 }
diff --git a/setjmp/tst-sigsetjmp2.c b/setjmp/tst-sigsetjmp2.c
new file mode 100644
index 0000000000..131531feb0
--- /dev/null
+++ b/setjmp/tst-sigsetjmp2.c
@@ -0,0 +1,50 @@
+/* Test that sigprocmask does not read from the unused part of jmpbuf.
+   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/>.  */
+
+#include <stdlib.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <jmp_buf-macros.h>
+#include <support/next_to_fault.h>
+
+static int
+do_test (void)
+{
+  struct support_next_to_fault jmpbuf
+    = support_next_to_fault_allocate (SAVED_MASK_OFFSET + (_NSIG / 8));
+  struct __jmp_buf_tag *sj = (struct __jmp_buf_tag *) jmpbuf.buffer;
+
+  errno = 0;
+  if (sigsetjmp (sj, 1) == 0)
+    {
+      siglongjmp (sj, 1);
+      return EXIT_FAILURE;
+    }
+  if (errno != 0)
+    {
+      printf ("sigsetjmp: %s\n", strerror (errno));
+      return EXIT_FAILURE;
+    }
+  return EXIT_SUCCESS;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/sysdeps/generic/__jmpbuf_arch_t.h b/sysdeps/generic/__jmpbuf_arch_t.h
new file mode 100644
index 0000000000..b663202a82
--- /dev/null
+++ b/sysdeps/generic/__jmpbuf_arch_t.h
@@ -0,0 +1,24 @@
+/* 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/>.  */
+
+#include <bits/types/__sigset_t.h>
+
+typedef struct
+  {
+    __sigset_t __saved_mask;
+  } __jmpbuf_arch_t;
diff --git a/sysdeps/generic/__saved_mask.h b/sysdeps/generic/__saved_mask.h
new file mode 100644
index 0000000000..5a59d5f385
--- /dev/null
+++ b/sysdeps/generic/__saved_mask.h
@@ -0,0 +1,20 @@
+/* Generic __saved_mask 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/>.  */
+
+/* Saved signal mask.  */
+#define __saved_mask __saved_mask.__saved_mask
diff --git a/sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h b/sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h
new file mode 100644
index 0000000000..e2372e898b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/__jmpbuf_arch_t.h
@@ -0,0 +1,43 @@
+/* __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/>.  */
+
+#include <bits/types/__sigset_t.h>
+
+/* The biggest signal number + 1.  As of kernel 4.13, the maximum number
+   of signals is 128 on MIPS.  Define it to 257 to leave some rooms for
+   the future.  */
+#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 architecture specific usage.  */
+	unsigned long int __padding[12];
+      } __saved;
+  } __jmpbuf_arch_t;
diff --git a/sysdeps/unix/sysv/linux/__saved_mask.h b/sysdeps/unix/sysv/linux/__saved_mask.h
new file mode 100644
index 0000000000..fcc7c26c9b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/__saved_mask.h
@@ -0,0 +1,20 @@
+/* __saved_mask 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/>.  */
+
+/* Saved signal mask.  */
+#define __saved_mask __saved_mask.__saved.__saved_mask
diff --git a/sysdeps/unix/sysv/linux/s390/longjmp_chk.c b/sysdeps/unix/sysv/linux/s390/longjmp_chk.c
index 878cbd4850..5e474bb6d2 100644
--- a/sysdeps/unix/sysv/linux/s390/longjmp_chk.c
+++ b/sysdeps/unix/sysv/linux/s390/longjmp_chk.c
@@ -24,7 +24,7 @@
 /* this is a copy from debug/longjmp_chk.c because we need an unique name
    for __longjmp_chk, but it is already named via a define
    for __libc_siglongjmp in debug/longjmp_chk.c.  */
-# include <setjmp.h>
+# include <setjmpP.h>
 
 // XXX Should move to include/setjmp.h
 extern void ____longjmp_chk (__jmp_buf __env, int __val)
-- 
2.13.6


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]