* V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
@ 2018-07-30 18:23 H.J. Lu
2018-07-30 18:49 ` Adhemerval Zanella
0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2018-07-30 18:23 UTC (permalink / raw)
To: Florian Weimer, GNU C Library, Carlos O'Donell
[-- Attachment #1: Type: text/plain, Size: 4658 bytes --]
On Sun, Jul 29, 2018 at 5:50 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jul 27, 2018 at 09:49:53PM -0700, H.J. Lu wrote:
>> On Fri, Jul 27, 2018 at 1:39 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> > On 07/27/2018 08:47 PM, H.J. Lu wrote:
>> >>
>> >> On Fri, Jul 27, 2018 at 11:26 AM, Florian Weimer <fweimer@redhat.com>
>> >> wrote:
>> >>>
>> >>> On 07/27/2018 08:22 PM, H.J. Lu wrote:
>> >>>>
>> >>>>
>> >>>> - while (1)
>> >>>> + while (ptr < ptr_end)
>> >>>> {
>> >>>> unsigned int type = *(unsigned int *) ptr;
>> >>>> unsigned int datasz = *(unsigned int *) (ptr + 4);
>> >>>
>> >>>
>> >>>
>> >>> You need 1 byte, but 8 bytes. Why is checking for at least 1 byte
>> >>> sufficient here?
>> >>>
>> >>
>> >> There is:
>> >>
>> >> /* Check for invalid property. */
>> >> if (note->n_descsz < 8
>> >> || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
>> >> break;
>> >>
>> >> before that. n_descsz should be correct.
>> >
>> >
>> > I do not have a strong opinion regarding this matter. For correctly
>> > generated notes, your patch should be fine.
>> >
>>
>> There is a real bug in the note parser. It doesn't check each item. This
>> test should fail on CET SDV since IBT is on and endbr64 is missing
>> in foo. But it passed:
>>
>> [hjl@gnu-cet-1 bad-property-1]$ cat y.S
>> #include <cet.h>
>>
>> .text
>> .globl main
>> .type main, @function
>> main:
>> .cfi_startproc
>> endbr64
>> subq $8, %rsp
>> .cfi_def_cfa_offset 16
>> movq $foo, %rdi
>> call *%rdi
>> xorl %eax, %eax
>> addq $8, %rsp
>> .cfi_def_cfa_offset 8
>> ret
>> .cfi_endproc
>> .size main, .-main
>> .p2align 4,,15
>> .type foo, @function
>> foo:
>> .cfi_startproc
>> ret
>> .cfi_endproc
>> .size foo, .-foo
>>
>> #if __SIZEOF_PTRDIFF_T__ == 8
>> # define ALIGN 3
>> #elif __SIZEOF_PTRDIFF_T__ == 4
>> # define ALIGN 2
>> #endif
>>
>> .section ".note.gnu.property", "a"
>> .p2align ALIGN
>>
>> .long 1f - 0f /* name length */
>> .long 5f - 2f /* data length */
>> .long 5 /* note type */
>> 0: .asciz "GNU" /* vendor name */
>> 1:
>> .p2align ALIGN
>> 2:
>> .long 1 /* pr_type. */
>> .long 4f - 3f /* pr_datasz. */
>> 3:
>> .long 0x800
>> .long 0x800
>> 4:
>> .p2align ALIGN
>> 5:
>>
>> .section .note.GNU-stack,"",@progbits
>> [hjl@gnu-cet-1 bad-property-1]$ make y
>> gcc -fcf-protection -c -o y.o y.S
>> gcc -fcf-protection -g -o y y.o
>> [hjl@gnu-cet-1 build-x86_64-linux]$
>> ../../glibc-cet-O3/build-x86_64-linux/elf/ld.so --library-path .
>> ~/bugs/libc/bad-property-1/y
>> [hjl@gnu-cet-1 build-x86_64-linux]$
>>
>> This patch fixes it by properly checking each item and stops searching
>> when a GNU_PROPERTY_X86_FEATURE_1_AND property is found
>> regardless if its size is correct or not. Linker won't generate bad
>> GNU_PROPERTY_X86_FEATURE_1_AND. But people may do crazy
>> things.
>>
>> Starting program:
>> /export/build/gnu/tools-build/glibc-cet/build-x86_64-linux/elf/ld.so
>> --library-path . ~/bugs/libc/bad-property-1/y
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x0000000000401130 in ?? ()
>> (gdb) disass 0x0000000000401130,+30
>> Dump of assembler code from 0x401130 to 0x40114e:
>> => 0x0000000000401130: retq
>> 0x0000000000401131: nopw %cs:0x0(%rax,%rax,1)
>> 0x000000000040113b: nopl 0x0(%rax,%rax,1)
>> 0x0000000000401140: endbr64
>> 0x0000000000401144: push %r15
>> 0x0000000000401146: mov %rdx,%r15
>> 0x0000000000401149: push %r14
>> 0x000000000040114b: mov %rsi,%r14
>> End of assembler dump.
>> (gdb)
>>
>> OK for master?
>>
>> Thanks.
>>
>> --
>> H.J.
>
>> From 75052a7f08a4261eb7c56885b56970ca96301d36 Mon Sep 17 00:00:00 2001
>> From: "H.J. Lu" <hjl.tools@gmail.com>
>> Date: Fri, 27 Jul 2018 20:34:55 -0700
>> Subject: [PATCH] x86/CET: Fix property note parser
>>
>> GNU_PROPERTY_X86_FEATURE_1_AND may not be the first property item. We
>> need to properly check each property item until we reach the end of the
>> property or find GNU_PROPERTY_X86_FEATURE_1_AND.
>>
>> * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Parse
>> each property item.
>
>
> The updated patch. We should return immediately when we find
> GNU_PROPERTY_X86_FEATURE_1_AND. There is no need to search further.
>
> OK for master?
>
Here is the updated patch with a testcase. I'd like to get it fixd
for 2.28.
--
H.J.
[-- Attachment #2: 0001-x86-CET-Fix-property-note-parser-BZ-23467.patch --]
[-- Type: text/x-patch, Size: 9318 bytes --]
From 7107d4fb27e04d5fbd939c648e69acfa88789a13 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 27 Jul 2018 20:34:55 -0700
Subject: [PATCH] x86/CET: Fix property note parser [BZ #23467]
GNU_PROPERTY_X86_FEATURE_1_AND may not be the first property item. We
need to check each property item until we reach the end of the property
or find GNU_PROPERTY_X86_FEATURE_1_AND.
This patch adds 2 tests. The first test checks if IBT is enabled and
the second test reads the output from the first test to check if IBT
is is enabled. The second second test fails if IBT isn't enabled
properly.
[BZ #23467]
* sysdeps/unix/sysv/linux/x86/Makefile (tests): Add
tst-cet-property-1 and tst-cet-property-2 if CET is enabled.
(CFLAGS-tst-cet-property-1.o): New.
(ASFLAGS-tst-cet-property-dep-2.o): Likewise.
($(objpfx)tst-cet-property-2): Likewise.
($(objpfx)tst-cet-property-2.out): Likewise.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c: New file.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c: Likewise.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S: Likewise.
* sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Parse
each property item until GNU_PROPERTY_X86_FEATURE_1_AND is
found.
---
sysdeps/unix/sysv/linux/x86/Makefile | 15 +++++
.../unix/sysv/linux/x86/tst-cet-property-1.c | 40 ++++++++++++
.../unix/sysv/linux/x86/tst-cet-property-2.c | 63 +++++++++++++++++++
.../sysv/linux/x86/tst-cet-property-dep-2.S | 60 ++++++++++++++++++
sysdeps/x86/dl-prop.h | 24 ++++---
5 files changed, 193 insertions(+), 9 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile
index a30fdb1dc1..7dc4e61756 100644
--- a/sysdeps/unix/sysv/linux/x86/Makefile
+++ b/sysdeps/unix/sysv/linux/x86/Makefile
@@ -25,6 +25,21 @@ tests += tst-saved_mask-1
endif
ifeq ($(enable-cet),yes)
+ifeq ($(subdir),elf)
+tests += tst-cet-property-1 tst-cet-property-2
+
+CFLAGS-tst-cet-property-1.o += -fcf-protection
+ASFLAGS-tst-cet-property-dep-2.o += -fcf-protection
+
+$(objpfx)tst-cet-property-2: $(objpfx)tst-cet-property-dep-2.o
+$(objpfx)tst-cet-property-2.out: $(objpfx)tst-cet-property-2 \
+ $(objpfx)tst-cet-property-1.out
+ env $(run-program-env) $(test-via-rtld-prefix) \
+ $(objpfx)tst-cet-property-2 \
+ < $(objpfx)tst-cet-property-1.out > $@; \
+ $(evaluate-test)
+endif
+
ifeq ($(subdir),stdlib)
tests += tst-cet-setcontext-1
CFLAGS-tst-cet-setcontext-1.c += -mshstk
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
new file mode 100644
index 0000000000..0a1e155770
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
@@ -0,0 +1,40 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <stdio.h>
+#include <elf.h>
+#include <tcb-offsets.h>
+
+static int
+do_test (void)
+{
+ unsigned int feature_1;
+#ifdef __x86_64__
+# define SEG_REG "fs"
+#else
+# define SEG_REG "gs"
+#endif
+ asm ("movl %%" SEG_REG ":%P1, %0"
+ : "=r" (feature_1) : "i" (FEATURE_1_OFFSET));
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
+ printf ("IBT\n");
+
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
new file mode 100644
index 0000000000..465f4f66e8
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
@@ -0,0 +1,63 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <stdio.h>
+#include <string.h>
+#include <signal.h>
+
+static int do_test (void);
+
+#include <support/test-driver.c>
+
+extern void bar (void);
+
+void
+__attribute__ ((noclone, noinline))
+test (void (*func_p) (void))
+{
+ func_p ();
+}
+
+static void
+sig_handler (int signo)
+{
+ exit (EXIT_SUCCESS);
+}
+
+static int
+do_test (void)
+{
+ char buf[20];
+
+ if (scanf ("%20s", buf) != 1)
+ return EXIT_UNSUPPORTED;
+
+ if (strcmp (buf, "IBT") != 0)
+ return EXIT_UNSUPPORTED;
+
+ if (signal (SIGSEGV, &sig_handler) == SIG_ERR)
+ {
+ perror ("installing SIGSEGV handler");
+ return EXIT_FAILURE;
+ }
+
+ test (bar);
+
+ return EXIT_FAILURE;
+}
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
new file mode 100644
index 0000000000..831a8b2e68
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
@@ -0,0 +1,60 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <cet.h>
+
+ .text
+ .p2align 4,,15
+ .globl bar
+ .type bar, @function
+bar:
+ .cfi_startproc
+ ret
+ .cfi_endproc
+ .size bar, .-bar
+
+#if __SIZEOF_PTRDIFF_T__ == 8
+# define ALIGN 3
+#elif __SIZEOF_PTRDIFF_T__ == 4
+# define ALIGN 2
+#endif
+
+ .section ".note.gnu.property", "a"
+ .p2align ALIGN
+
+ .long 1f - 0f /* name length */
+ .long 5f - 2f /* data length */
+ .long 5 /* note type */
+0: .asciz "GNU" /* vendor name */
+1:
+ .p2align ALIGN
+2:
+ .long 1 /* pr_type. */
+ .long 4f - 3f /* pr_datasz. */
+3:
+#if __SIZEOF_PTRDIFF_T__ == 8
+ .long 0x800
+ .long 0x800
+#else
+ .long 0x08000800
+#endif
+4:
+ .p2align ALIGN
+5:
+
+ .section .note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 35d3f16a23..d9e0770e29 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
unsigned char *ptr = (unsigned char *) (note + 1) + 4;
unsigned char *ptr_end = ptr + note->n_descsz;
- while (ptr < ptr_end)
+ do
{
unsigned int type = *(unsigned int *) ptr;
unsigned int datasz = *(unsigned int *) (ptr + 4);
@@ -82,17 +82,23 @@ _dl_process_cet_property_note (struct link_map *l,
if ((ptr + datasz) > ptr_end)
break;
- if (type == GNU_PROPERTY_X86_FEATURE_1_AND
- && datasz == 4)
+ if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
{
- unsigned int feature_1 = *(unsigned int *) ptr;
- if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
- l->l_cet |= lc_ibt;
- if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
- l->l_cet |= lc_shstk;
- break;
+ if (datasz == 4)
+ {
+ unsigned int feature_1 = *(unsigned int *) ptr;
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
+ l->l_cet |= lc_ibt;
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+ l->l_cet |= lc_shstk;
+ }
+ return;
}
+
+ /* Check the next property item. */
+ ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
}
+ while ((ptr_end - ptr) >= 8);
}
/* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
2018-07-30 18:23 V3 [PATCH] x86/CET: Fix property note parser [BZ #23467] H.J. Lu
@ 2018-07-30 18:49 ` Adhemerval Zanella
2018-07-30 18:56 ` H.J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella @ 2018-07-30 18:49 UTC (permalink / raw)
To: libc-alpha
On 30/07/2018 15:23, H.J. Lu wrote:
> diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
> new file mode 100644
> index 0000000000..465f4f66e8
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
> +
> +static int
> +do_test (void)
> +{
> + char buf[20];
> +
> + if (scanf ("%20s", buf) != 1)
> + return EXIT_UNSUPPORTED;
> +
> + if (strcmp (buf, "IBT") != 0)
> + return EXIT_UNSUPPORTED;
Maybe use FAIL_UNSUPPORTED (...) ?
> +
> + if (signal (SIGSEGV, &sig_handler) == SIG_ERR)
> + {
> + perror ("installing SIGSEGV handler");
> + return EXIT_FAILURE;
> + }
Maybe use TEST_VERIFY_EXIT (signal (SIGSEGV, &sig_handler) != SIG_ERR) ?
> +
> + test (bar);
> +
> + return EXIT_FAILURE;
> +}
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 35d3f16a23..d9e0770e29 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> unsigned char *ptr_end = ptr + note->n_descsz;
>
Should we care for overflow here (I guess not since we don't really
protected against ill-formed elf files in general)?
> - while (ptr < ptr_end)
> + do
> {
> unsigned int type = *(unsigned int *) ptr;
> unsigned int datasz = *(unsigned int *) (ptr + 4);
> @@ -82,17 +82,23 @@ _dl_process_cet_property_note (struct link_map *l,
> if ((ptr + datasz) > ptr_end)
> break;
>
> - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> - && datasz == 4)
> + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> {
> - unsigned int feature_1 = *(unsigned int *) ptr;
> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
> - l->l_cet |= lc_ibt;
> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
> - l->l_cet |= lc_shstk;
> - break;
> + if (datasz == 4)
> + {
> + unsigned int feature_1 = *(unsigned int *) ptr;
> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
> + l->l_cet |= lc_ibt;
> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
> + l->l_cet |= lc_shstk;
> + }
> + return;
> }
> +
> + /* Check the next property item. */
> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> }
> + while ((ptr_end - ptr) >= 8);
> }
>
> /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
> -- 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
2018-07-30 18:49 ` Adhemerval Zanella
@ 2018-07-30 18:56 ` H.J. Lu
2018-07-30 19:10 ` Adhemerval Zanella
0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2018-07-30 18:56 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Mon, Jul 30, 2018 at 11:48 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 30/07/2018 15:23, H.J. Lu wrote:
>> diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
>> new file mode 100644
>> index 0000000000..465f4f66e8
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
>
>
>> +
>> +static int
>> +do_test (void)
>> +{
>> + char buf[20];
>> +
>> + if (scanf ("%20s", buf) != 1)
>> + return EXIT_UNSUPPORTED;
>> +
>> + if (strcmp (buf, "IBT") != 0)
>> + return EXIT_UNSUPPORTED;
>
> Maybe use FAIL_UNSUPPORTED (...) ?
Good point.
>
>> +
>> + if (signal (SIGSEGV, &sig_handler) == SIG_ERR)
>> + {
>> + perror ("installing SIGSEGV handler");
>> + return EXIT_FAILURE;
>> + }
>
> Maybe use TEST_VERIFY_EXIT (signal (SIGSEGV, &sig_handler) != SIG_ERR) ?
Good point.
>> +
>> + test (bar);
>> +
>> + return EXIT_FAILURE;
>> +}
>
>> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
>> index 35d3f16a23..d9e0770e29 100644
>> --- a/sysdeps/x86/dl-prop.h
>> +++ b/sysdeps/x86/dl-prop.h
>> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
>> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
>> unsigned char *ptr_end = ptr + note->n_descsz;
>>
>
> Should we care for overflow here (I guess not since we don't really
> protected against ill-formed elf files in general)?
We do protect against ill-formed notes. When we get here, the whole
note has been loaded into memory. There won't be overflow.
>> - while (ptr < ptr_end)
>> + do
>> {
>> unsigned int type = *(unsigned int *) ptr;
>> unsigned int datasz = *(unsigned int *) (ptr + 4);
>> @@ -82,17 +82,23 @@ _dl_process_cet_property_note (struct link_map *l,
>> if ((ptr + datasz) > ptr_end)
>> break;
>>
>> - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
>> - && datasz == 4)
>> + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
>> {
>> - unsigned int feature_1 = *(unsigned int *) ptr;
>> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
>> - l->l_cet |= lc_ibt;
>> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
>> - l->l_cet |= lc_shstk;
>> - break;
>> + if (datasz == 4)
>> + {
>> + unsigned int feature_1 = *(unsigned int *) ptr;
>> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
>> + l->l_cet |= lc_ibt;
>> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
>> + l->l_cet |= lc_shstk;
>> + }
>> + return;
>> }
>> +
>> + /* Check the next property item. */
>> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
>> }
>> + while ((ptr_end - ptr) >= 8);
>> }
>>
>> /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
>> -- 2.17.1
>>
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
2018-07-30 18:56 ` H.J. Lu
@ 2018-07-30 19:10 ` Adhemerval Zanella
2018-07-30 19:34 ` H.J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella @ 2018-07-30 19:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 30/07/2018 15:56, H.J. Lu wrote:
>>> +
>>> + test (bar);
>>> +
>>> + return EXIT_FAILURE;
>>> +}
>>
>>> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
>>> index 35d3f16a23..d9e0770e29 100644
>>> --- a/sysdeps/x86/dl-prop.h
>>> +++ b/sysdeps/x86/dl-prop.h
>>> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
>>> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
>>> unsigned char *ptr_end = ptr + note->n_descsz;
>>>
>>
>> Should we care for overflow here (I guess not since we don't really
>> protected against ill-formed elf files in general)?
>
> We do protect against ill-formed notes. When we get here, the whole
> note has been loaded into memory. There won't be overflow.
Indeed, LGTM to me then.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
2018-07-30 19:10 ` Adhemerval Zanella
@ 2018-07-30 19:34 ` H.J. Lu
2018-07-30 19:43 ` Carlos O'Donell
0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2018-07-30 19:34 UTC (permalink / raw)
To: Adhemerval Zanella, Carlos O'Donell; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 963 bytes --]
On Mon, Jul 30, 2018 at 12:09 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 30/07/2018 15:56, H.J. Lu wrote:
>>>> +
>>>> + test (bar);
>>>> +
>>>> + return EXIT_FAILURE;
>>>> +}
>>>
>>>> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
>>>> index 35d3f16a23..d9e0770e29 100644
>>>> --- a/sysdeps/x86/dl-prop.h
>>>> +++ b/sysdeps/x86/dl-prop.h
>>>> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
>>>> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
>>>> unsigned char *ptr_end = ptr + note->n_descsz;
>>>>
>>>
>>> Should we care for overflow here (I guess not since we don't really
>>> protected against ill-formed elf files in general)?
>>
>> We do protect against ill-formed notes. When we get here, the whole
>> note has been loaded into memory. There won't be overflow.
>
> Indeed, LGTM to me then.
This is the updated patch I am going to check in today.
Thanks.
--
H.J.
[-- Attachment #2: 0001-x86-CET-Fix-property-note-parser-BZ-23467.patch --]
[-- Type: text/x-patch, Size: 9271 bytes --]
From bed8cb21b6bd67bed148713259779a8a3e3f844d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 27 Jul 2018 20:34:55 -0700
Subject: [PATCH] x86/CET: Fix property note parser [BZ #23467]
GNU_PROPERTY_X86_FEATURE_1_AND may not be the first property item. We
need to check each property item until we reach the end of the property
or find GNU_PROPERTY_X86_FEATURE_1_AND.
This patch adds 2 tests. The first test checks if IBT is enabled and
the second test reads the output from the first test to check if IBT
is is enabled. The second second test fails if IBT isn't enabled
properly.
[BZ #23467]
* sysdeps/unix/sysv/linux/x86/Makefile (tests): Add
tst-cet-property-1 and tst-cet-property-2 if CET is enabled.
(CFLAGS-tst-cet-property-1.o): New.
(ASFLAGS-tst-cet-property-dep-2.o): Likewise.
($(objpfx)tst-cet-property-2): Likewise.
($(objpfx)tst-cet-property-2.out): Likewise.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c: New file.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c: Likewise.
* sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S: Likewise.
* sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Parse
each property item until GNU_PROPERTY_X86_FEATURE_1_AND is
found.
---
sysdeps/unix/sysv/linux/x86/Makefile | 15 +++++
.../unix/sysv/linux/x86/tst-cet-property-1.c | 40 +++++++++++++
.../unix/sysv/linux/x86/tst-cet-property-2.c | 58 ++++++++++++++++++
.../sysv/linux/x86/tst-cet-property-dep-2.S | 60 +++++++++++++++++++
sysdeps/x86/dl-prop.h | 24 +++++---
5 files changed, 188 insertions(+), 9 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile
index a30fdb1dc1..7dc4e61756 100644
--- a/sysdeps/unix/sysv/linux/x86/Makefile
+++ b/sysdeps/unix/sysv/linux/x86/Makefile
@@ -25,6 +25,21 @@ tests += tst-saved_mask-1
endif
ifeq ($(enable-cet),yes)
+ifeq ($(subdir),elf)
+tests += tst-cet-property-1 tst-cet-property-2
+
+CFLAGS-tst-cet-property-1.o += -fcf-protection
+ASFLAGS-tst-cet-property-dep-2.o += -fcf-protection
+
+$(objpfx)tst-cet-property-2: $(objpfx)tst-cet-property-dep-2.o
+$(objpfx)tst-cet-property-2.out: $(objpfx)tst-cet-property-2 \
+ $(objpfx)tst-cet-property-1.out
+ env $(run-program-env) $(test-via-rtld-prefix) \
+ $(objpfx)tst-cet-property-2 \
+ < $(objpfx)tst-cet-property-1.out > $@; \
+ $(evaluate-test)
+endif
+
ifeq ($(subdir),stdlib)
tests += tst-cet-setcontext-1
CFLAGS-tst-cet-setcontext-1.c += -mshstk
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
new file mode 100644
index 0000000000..0a1e155770
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
@@ -0,0 +1,40 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <stdio.h>
+#include <elf.h>
+#include <tcb-offsets.h>
+
+static int
+do_test (void)
+{
+ unsigned int feature_1;
+#ifdef __x86_64__
+# define SEG_REG "fs"
+#else
+# define SEG_REG "gs"
+#endif
+ asm ("movl %%" SEG_REG ":%P1, %0"
+ : "=r" (feature_1) : "i" (FEATURE_1_OFFSET));
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
+ printf ("IBT\n");
+
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
new file mode 100644
index 0000000000..fe793e92b6
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
@@ -0,0 +1,58 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <stdio.h>
+#include <string.h>
+#include <signal.h>
+#include <support/check.h>
+
+extern void bar (void);
+
+void
+__attribute__ ((noclone, noinline))
+test (void (*func_p) (void))
+{
+ func_p ();
+}
+
+static void
+sig_handler (int signo)
+{
+ exit (EXIT_SUCCESS);
+}
+
+static int
+do_test (void)
+{
+ char buf[20];
+
+ if (scanf ("%20s", buf) != 1)
+ FAIL_UNSUPPORTED ("IBT not supported");
+
+ if (strcmp (buf, "IBT") != 0)
+ FAIL_UNSUPPORTED ("IBT not supported");
+
+ TEST_VERIFY_EXIT (signal (SIGSEGV, &sig_handler) != SIG_ERR);
+
+ test (bar);
+
+ return EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
new file mode 100644
index 0000000000..831a8b2e68
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
@@ -0,0 +1,60 @@
+/* Test CET property note parser.
+ Copyright (C) 2018 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 <cet.h>
+
+ .text
+ .p2align 4,,15
+ .globl bar
+ .type bar, @function
+bar:
+ .cfi_startproc
+ ret
+ .cfi_endproc
+ .size bar, .-bar
+
+#if __SIZEOF_PTRDIFF_T__ == 8
+# define ALIGN 3
+#elif __SIZEOF_PTRDIFF_T__ == 4
+# define ALIGN 2
+#endif
+
+ .section ".note.gnu.property", "a"
+ .p2align ALIGN
+
+ .long 1f - 0f /* name length */
+ .long 5f - 2f /* data length */
+ .long 5 /* note type */
+0: .asciz "GNU" /* vendor name */
+1:
+ .p2align ALIGN
+2:
+ .long 1 /* pr_type. */
+ .long 4f - 3f /* pr_datasz. */
+3:
+#if __SIZEOF_PTRDIFF_T__ == 8
+ .long 0x800
+ .long 0x800
+#else
+ .long 0x08000800
+#endif
+4:
+ .p2align ALIGN
+5:
+
+ .section .note.GNU-stack,"",@progbits
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 35d3f16a23..d9e0770e29 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
unsigned char *ptr = (unsigned char *) (note + 1) + 4;
unsigned char *ptr_end = ptr + note->n_descsz;
- while (ptr < ptr_end)
+ do
{
unsigned int type = *(unsigned int *) ptr;
unsigned int datasz = *(unsigned int *) (ptr + 4);
@@ -82,17 +82,23 @@ _dl_process_cet_property_note (struct link_map *l,
if ((ptr + datasz) > ptr_end)
break;
- if (type == GNU_PROPERTY_X86_FEATURE_1_AND
- && datasz == 4)
+ if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
{
- unsigned int feature_1 = *(unsigned int *) ptr;
- if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
- l->l_cet |= lc_ibt;
- if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
- l->l_cet |= lc_shstk;
- break;
+ if (datasz == 4)
+ {
+ unsigned int feature_1 = *(unsigned int *) ptr;
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
+ l->l_cet |= lc_ibt;
+ if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+ l->l_cet |= lc_shstk;
+ }
+ return;
}
+
+ /* Check the next property item. */
+ ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
}
+ while ((ptr_end - ptr) >= 8);
}
/* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: V3 [PATCH] x86/CET: Fix property note parser [BZ #23467]
2018-07-30 19:34 ` H.J. Lu
@ 2018-07-30 19:43 ` Carlos O'Donell
0 siblings, 0 replies; 6+ messages in thread
From: Carlos O'Donell @ 2018-07-30 19:43 UTC (permalink / raw)
To: H.J. Lu, Adhemerval Zanella; +Cc: GNU C Library
On 07/30/2018 03:34 PM, H.J. Lu wrote:
> On Mon, Jul 30, 2018 at 12:09 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> On 30/07/2018 15:56, H.J. Lu wrote:
>>>>> +
>>>>> + test (bar);
>>>>> +
>>>>> + return EXIT_FAILURE;
>>>>> +}
>>>>> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
>>>>> index 35d3f16a23..d9e0770e29 100644
>>>>> --- a/sysdeps/x86/dl-prop.h
>>>>> +++ b/sysdeps/x86/dl-prop.h
>>>>> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
>>>>> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
>>>>> unsigned char *ptr_end = ptr + note->n_descsz;
>>>>>
>>>> Should we care for overflow here (I guess not since we don't really
>>>> protected against ill-formed elf files in general)?
>>> We do protect against ill-formed notes. When we get here, the whole
>>> note has been loaded into memory. There won't be overflow.
>> Indeed, LGTM to me then.
> This is the updated patch I am going to check in today.
Please get final Reviewed-by from the release manager for any patches
which are materially different from those reviewed.
> From bed8cb21b6bd67bed148713259779a8a3e3f844d Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Fri, 27 Jul 2018 20:34:55 -0700
> Subject: [PATCH] x86/CET: Fix property note parser [BZ #23467]
>
> GNU_PROPERTY_X86_FEATURE_1_AND may not be the first property item. We
> need to check each property item until we reach the end of the property
> or find GNU_PROPERTY_X86_FEATURE_1_AND.
>
> This patch adds 2 tests. The first test checks if IBT is enabled and
> the second test reads the output from the first test to check if IBT
> is is enabled. The second second test fails if IBT isn't enabled
> properly.
>
> [BZ #23467]
> * sysdeps/unix/sysv/linux/x86/Makefile (tests): Add
> tst-cet-property-1 and tst-cet-property-2 if CET is enabled.
> (CFLAGS-tst-cet-property-1.o): New.
> (ASFLAGS-tst-cet-property-dep-2.o): Likewise.
> ($(objpfx)tst-cet-property-2): Likewise.
> ($(objpfx)tst-cet-property-2.out): Likewise.
> * sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c: New file.
> * sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c: Likewise.
> * sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S: Likewise.
> * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Parse
> each property item until GNU_PROPERTY_X86_FEATURE_1_AND is
> found.
> ---
> sysdeps/unix/sysv/linux/x86/Makefile | 15 +++++
> .../unix/sysv/linux/x86/tst-cet-property-1.c | 40 +++++++++++++
> .../unix/sysv/linux/x86/tst-cet-property-2.c | 58 ++++++++++++++++++
> .../sysv/linux/x86/tst-cet-property-dep-2.S | 60 +++++++++++++++++++
> sysdeps/x86/dl-prop.h | 24 +++++---
> 5 files changed, 188 insertions(+), 9 deletions(-)
> create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
> create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
> create mode 100644 sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
OK for 2.28 with 5 comment additions.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile
> index a30fdb1dc1..7dc4e61756 100644
> --- a/sysdeps/unix/sysv/linux/x86/Makefile
> +++ b/sysdeps/unix/sysv/linux/x86/Makefile
> @@ -25,6 +25,21 @@ tests += tst-saved_mask-1
> endif
>
> ifeq ($(enable-cet),yes)
> +ifeq ($(subdir),elf)
> +tests += tst-cet-property-1 tst-cet-property-2
> +
> +CFLAGS-tst-cet-property-1.o += -fcf-protection
> +ASFLAGS-tst-cet-property-dep-2.o += -fcf-protection
OK.
> +
> +$(objpfx)tst-cet-property-2: $(objpfx)tst-cet-property-dep-2.o
> +$(objpfx)tst-cet-property-2.out: $(objpfx)tst-cet-property-2 \
> + $(objpfx)tst-cet-property-1.out
> + env $(run-program-env) $(test-via-rtld-prefix) \
> + $(objpfx)tst-cet-property-2 \
> + < $(objpfx)tst-cet-property-1.out > $@; \
> + $(evaluate-test)
OK.
> +endif
> +
> ifeq ($(subdir),stdlib)
> tests += tst-cet-setcontext-1
> CFLAGS-tst-cet-setcontext-1.c += -mshstk
> diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
> new file mode 100644
> index 0000000000..0a1e155770
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-1.c
> @@ -0,0 +1,40 @@
> +/* Test CET property note parser.
OK.
> + Copyright (C) 2018 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 <stdio.h>
> +#include <elf.h>
> +#include <tcb-offsets.h>
> +
(1)
Needs a comment explaining why this test runs first before the
second test and what it should output.
> +static int
> +do_test (void)
> +{
> + unsigned int feature_1;
> +#ifdef __x86_64__
> +# define SEG_REG "fs"
> +#else
> +# define SEG_REG "gs"
> +#endif
> + asm ("movl %%" SEG_REG ":%P1, %0"
> + : "=r" (feature_1) : "i" (FEATURE_1_OFFSET));
> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0)
> + printf ("IBT\n");
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
> new file mode 100644
> index 0000000000..fe793e92b6
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-2.c
> @@ -0,0 +1,58 @@
> +/* Test CET property note parser.
(2)
The core test should reference bug # please.
> + Copyright (C) 2018 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 <stdio.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <support/check.h>
> +
> +extern void bar (void);
> +
> +void
> +__attribute__ ((noclone, noinline))
> +test (void (*func_p) (void))
> +{
> + func_p ();
> +}
> +
> +static void
> +sig_handler (int signo)
> +{
> + exit (EXIT_SUCCESS);
> +}
> +
(3)
Needs a comment explaining what this is testing.
> +static int
> +do_test (void)
> +{
> + char buf[20];
> +
> + if (scanf ("%20s", buf) != 1)
> + FAIL_UNSUPPORTED ("IBT not supported");
OK.
> +
> + if (strcmp (buf, "IBT") != 0)
> + FAIL_UNSUPPORTED ("IBT not supported");
OK.
> +
> + TEST_VERIFY_EXIT (signal (SIGSEGV, &sig_handler) != SIG_ERR);
> +
> + test (bar);
> +
> + return EXIT_FAILURE;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
> new file mode 100644
> index 0000000000..831a8b2e68
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/x86/tst-cet-property-dep-2.S
> @@ -0,0 +1,60 @@
> +/* Test CET property note parser.
> + Copyright (C) 2018 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 <cet.h>
> +
> + .text
> + .p2align 4,,15
> + .globl bar
> + .type bar, @function
> +bar:
> + .cfi_startproc
> + ret
> + .cfi_endproc
> + .size bar, .-bar
> +
> +#if __SIZEOF_PTRDIFF_T__ == 8
> +# define ALIGN 3
> +#elif __SIZEOF_PTRDIFF_T__ == 4
> +# define ALIGN 2
> +#endif
> +
(4)
Needs a comment explaining this construct and what we are testing.
> + .section ".note.gnu.property", "a"
> + .p2align ALIGN
> +
> + .long 1f - 0f /* name length */
> + .long 5f - 2f /* data length */
> + .long 5 /* note type */
> +0: .asciz "GNU" /* vendor name */
> +1:
> + .p2align ALIGN
> +2:
> + .long 1 /* pr_type. */
> + .long 4f - 3f /* pr_datasz. */
> +3:
> +#if __SIZEOF_PTRDIFF_T__ == 8
> + .long 0x800
> + .long 0x800
> +#else
> + .long 0x08000800
> +#endif
> +4:
> + .p2align ALIGN
> +5:
> +
> + .section .note.GNU-stack,"",@progbits
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 35d3f16a23..d9e0770e29 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -73,7 +73,7 @@ _dl_process_cet_property_note (struct link_map *l,
> unsigned char *ptr = (unsigned char *) (note + 1) + 4;
> unsigned char *ptr_end = ptr + note->n_descsz;
>
> - while (ptr < ptr_end)
> + do
OK.
> {
> unsigned int type = *(unsigned int *) ptr;
> unsigned int datasz = *(unsigned int *) (ptr + 4);
> @@ -82,17 +82,23 @@ _dl_process_cet_property_note (struct link_map *l,
> if ((ptr + datasz) > ptr_end)
> break;
>
> - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> - && datasz == 4)
> + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
OK.
> {
> - unsigned int feature_1 = *(unsigned int *) ptr;
> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
> - l->l_cet |= lc_ibt;
> - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
> - l->l_cet |= lc_shstk;
> - break;
> + if (datasz == 4)
> + {
> + unsigned int feature_1 = *(unsigned int *) ptr;
> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
> + l->l_cet |= lc_ibt;
> + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
> + l->l_cet |= lc_shstk;
OK.
> + }
(5)
This needs a comment explaining why we return when datasz != 4.
> + return;
> }
> +
> + /* Check the next property item. */
> + ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
OK.
> }
> + while ((ptr_end - ptr) >= 8);
OK.
> }
>
> /* NB: Note sections like .note.ABI-tag and .note.gnu.build-id are
> -- 2.17.1
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-30 19:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 18:23 V3 [PATCH] x86/CET: Fix property note parser [BZ #23467] H.J. Lu
2018-07-30 18:49 ` Adhemerval Zanella
2018-07-30 18:56 ` H.J. Lu
2018-07-30 19:10 ` Adhemerval Zanella
2018-07-30 19:34 ` H.J. Lu
2018-07-30 19:43 ` Carlos O'Donell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).