[HELP,NEEDED] elf: Add elf/check-wx-segment, a test for the presence of WX segments

Message ID 87fterp1ad.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer March 2, 2020, 9:13 a.m. UTC
  * John David Anglin:

> On 2020-02-28 5:07 a.m., Florian Weimer wrote:
>> Would you please help me to add proper comments to the xfails for the
>> hppa and sparc ports?
>>
>> Currently, both (sets of) targets have an writable and executable load
>> segment.  Is this because the lack of secure PLT support, like
>> originally on POWER?

> Currently, the PLT is executable on hppa because it contains an
> executable trampoline used during lazy binding.

Thanks, I've updated the patch.

>> The new test passes everywhere else.
>>
> Attached are test results without your change and with your change,
> respectively.  Also attached is the output from the new test.  The
> test results were:
>
> XFAIL: elf/check-wx-segment
> original exit status 1

It's a cross-test, so it runs during build-many-glibcs.py, too.

Adhemerval, I've incorporated your comments as well.

Okay for master?

Thanks,
Florian
8<------------------------------------------------------------------8<
Writable, executable segments defeat security hardening.  The
existing check for DT_TEXTREL does not catch this.

hppa and SPARC currently keep the PLT in an RWX load segment.

-----
 elf/Makefile                          |  7 +++
 scripts/check-wx-segment.py           | 85 +++++++++++++++++++++++++++++++++++
 sysdeps/sparc/Makefile                |  9 ++++
 sysdeps/unix/sysv/linux/hppa/Makefile |  9 +++-
 4 files changed, 108 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella Netto March 2, 2020, 12:08 p.m. UTC | #1
On 02/03/2020 06:13, Florian Weimer wrote:
> * John David Anglin:
> 
>> On 2020-02-28 5:07 a.m., Florian Weimer wrote:
>>> Would you please help me to add proper comments to the xfails for the
>>> hppa and sparc ports?
>>>
>>> Currently, both (sets of) targets have an writable and executable load
>>> segment.  Is this because the lack of secure PLT support, like
>>> originally on POWER?
> 
>> Currently, the PLT is executable on hppa because it contains an
>> executable trampoline used during lazy binding.
> 
> Thanks, I've updated the patch.
> 
>>> The new test passes everywhere else.
>>>
>> Attached are test results without your change and with your change,
>> respectively.  Also attached is the output from the new test.  The
>> test results were:
>>
>> XFAIL: elf/check-wx-segment
>> original exit status 1
> 
> It's a cross-test, so it runs during build-many-glibcs.py, too.
> 
> Adhemerval, I've incorporated your comments as well.
> 
> Okay for master?

LGTM, thanks.

> 
> Thanks,
> Florian
> 8<------------------------------------------------------------------8<
> Writable, executable segments defeat security hardening.  The
> existing check for DT_TEXTREL does not catch this.
> 
> hppa and SPARC currently keep the PLT in an RWX load segment.
> 
> -----
>  elf/Makefile                          |  7 +++
>  scripts/check-wx-segment.py           | 85 +++++++++++++++++++++++++++++++++++
>  sysdeps/sparc/Makefile                |  9 ++++
>  sysdeps/unix/sysv/linux/hppa/Makefile |  9 +++-
>  4 files changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index a137143db7..da689a2c7b 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -402,6 +402,7 @@ tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out \
>  		 $(objpfx)tst-rtld-preload.out
>  endif
>  tests-special += $(objpfx)check-textrel.out $(objpfx)check-execstack.out \
> +		 $(objpfx)check-wx-segment.out \
>  		 $(objpfx)check-localplt.out $(objpfx)check-initfini.out
>  endif
>  
> @@ -1180,6 +1181,12 @@ $(objpfx)check-execstack.out: $(..)scripts/check-execstack.awk \
>  	$(evaluate-test)
>  generated += check-execstack.out
>  
> +$(objpfx)check-wx-segment.out: $(..)scripts/check-wx-segment.py \
> +			      $(all-built-dso:=.phdr)
> +	$(PYTHON) $^ --xfail="$(check-wx-segment-xfail)" > $@; \
> +	$(evaluate-test)
> +generated += check-wx-segment.out
> +
>  $(objpfx)tst-dlmodcount: $(libdl)
>  $(objpfx)tst-dlmodcount.out: $(test-modules)
>  
> diff --git a/scripts/check-wx-segment.py b/scripts/check-wx-segment.py
> new file mode 100644
> index 0000000000..e1fa79387c
> --- /dev/null
> +++ b/scripts/check-wx-segment.py
> @@ -0,0 +1,85 @@
> +#!/usr/bin/python3
> +# Check ELF program headers for WX segments.
> +# Copyright (C) 2020 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
> +# <https://www.gnu.org/licenses/>.
> +
> +"""Check that the program headers do not contain write-exec segments."""
> +
> +import argparse
> +import os.path
> +import re
> +import sys
> +
> +# Regular expression to extract the RWE flags field.  The
> +# address/offset columns have varying width.
> +RE_LOAD = re.compile(
> +    r'^  LOAD +(?:0x[0-9a-fA-F]+ +){5}([R ][W ][ E]) +0x[0-9a-fA-F]+\n\Z')
> +
> +def process_file(path, inp, xfail):
> +    """Analyze one input file."""
> +
> +    errors = 0
> +    for line in inp:
> +        error = None
> +        if line.startswith('  LOAD '):
> +            match = RE_LOAD.match(line)
> +            if match is None:
> +                error = 'Invalid LOAD line'
> +            else:
> +                flags, = match.groups()
> +                if 'W' in flags and 'E' in flags:
> +                    if xfail:
> +                        print('{}: warning: WX segment (as expected)'.format(
> +                            path))
> +                    else:
> +                        error = 'WX segment'
> +
> +        if error is not None:
> +            print('{}: error: {}: {!r}'.format(path, error, line.strip()))
> +            errors += 1
> +
> +    if xfail and errors == 0:
> +        print('{}: warning: missing expected WX segment'.format(path))
> +    return errors
> +
> +
> +def main():
> +    """The main entry point."""
> +    parser = argparse.ArgumentParser(description=__doc__)
> +    parser.add_argument('--xfail',
> +                        help='Mark input files as XFAILed ("*" for all)',
> +                        type=str, default='')
> +    parser.add_argument('phdrs',
> +                        help='Files containing readelf -Wl output',
> +                        nargs='*')
> +    opts = parser.parse_args(sys.argv)
> +
> +    xfails = set(opts.xfail.split(' '))
> +    xfails_all = opts.xfail.strip() == '*'
> +
> +    errors = 0
> +    for path in opts.phdrs:
> +        xfail = ((os.path.basename(path) + '.phdrs') in xfails
> +                 or xfails_all)
> +        with open(path) as inp:
> +            errors += process_file(path, inp, xfail)
> +    if errors > 0:
> +        sys.exit(1)
> +
> +
> +if __name__ == '__main__':
> +    main()
> diff --git a/sysdeps/sparc/Makefile b/sysdeps/sparc/Makefile
> index 38b33af6e0..c123b527b5 100644
> --- a/sysdeps/sparc/Makefile
> +++ b/sysdeps/sparc/Makefile
> @@ -16,6 +16,15 @@ CPPFLAGS-crti.S += -fPIC
>  CPPFLAGS-crtn.S += -fPIC
>  endif
>  
> +ifeq ($(subdir),elf)
> +
> +# Lazy binding on SPARC rewrites the PLT sequence.  See the Solaris
> +# Linker and Libraries Guide, section SPARC: Procedure Linkage Table.
> +# <https://docs.oracle.com/cd/E19455-01/816-0559/chapter6-1236/index.html>
> +test-xfail-check-wx-segment = *
> +
> +endif # $(subdir) == elf
> +
>  # nscd uses atomic_spin_nop which in turn requires cpu_relax
>  ifeq ($(subdir),nscd)
>  routines += cpu_relax
> diff --git a/sysdeps/unix/sysv/linux/hppa/Makefile b/sysdeps/unix/sysv/linux/hppa/Makefile
> index e1637f54f5..c89ec83182 100644
> --- a/sysdeps/unix/sysv/linux/hppa/Makefile
> +++ b/sysdeps/unix/sysv/linux/hppa/Makefile
> @@ -3,9 +3,14 @@ ifeq ($(subdir),stdlib)
>  gen-as-const-headers += ucontext_i.sym
>  endif
>  
> +ifeq ($(subdir),elf)
>  # Supporting non-executable stacks on HPPA requires changes to both
>  # the Linux kernel and glibc. The kernel currently needs an executable
>  # stack for syscall restarts and signal returns.
> -ifeq ($(subdir),elf)
>  test-xfail-check-execstack = yes
> -endif
> +
> +# On hppa, the PLT is executable because it contains an executable
> +# trampoline used during lazy binding.
> +test-xfail-check-wx-segment = *
> +
> +endif # $(subdir) == elf
>
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index a137143db7..da689a2c7b 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -402,6 +402,7 @@  tests-special += $(objpfx)tst-pathopt.out $(objpfx)tst-rtld-load-self.out \
 		 $(objpfx)tst-rtld-preload.out
 endif
 tests-special += $(objpfx)check-textrel.out $(objpfx)check-execstack.out \
+		 $(objpfx)check-wx-segment.out \
 		 $(objpfx)check-localplt.out $(objpfx)check-initfini.out
 endif
 
@@ -1180,6 +1181,12 @@  $(objpfx)check-execstack.out: $(..)scripts/check-execstack.awk \
 	$(evaluate-test)
 generated += check-execstack.out
 
+$(objpfx)check-wx-segment.out: $(..)scripts/check-wx-segment.py \
+			      $(all-built-dso:=.phdr)
+	$(PYTHON) $^ --xfail="$(check-wx-segment-xfail)" > $@; \
+	$(evaluate-test)
+generated += check-wx-segment.out
+
 $(objpfx)tst-dlmodcount: $(libdl)
 $(objpfx)tst-dlmodcount.out: $(test-modules)
 
diff --git a/scripts/check-wx-segment.py b/scripts/check-wx-segment.py
new file mode 100644
index 0000000000..e1fa79387c
--- /dev/null
+++ b/scripts/check-wx-segment.py
@@ -0,0 +1,85 @@ 
+#!/usr/bin/python3
+# Check ELF program headers for WX segments.
+# Copyright (C) 2020 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
+# <https://www.gnu.org/licenses/>.
+
+"""Check that the program headers do not contain write-exec segments."""
+
+import argparse
+import os.path
+import re
+import sys
+
+# Regular expression to extract the RWE flags field.  The
+# address/offset columns have varying width.
+RE_LOAD = re.compile(
+    r'^  LOAD +(?:0x[0-9a-fA-F]+ +){5}([R ][W ][ E]) +0x[0-9a-fA-F]+\n\Z')
+
+def process_file(path, inp, xfail):
+    """Analyze one input file."""
+
+    errors = 0
+    for line in inp:
+        error = None
+        if line.startswith('  LOAD '):
+            match = RE_LOAD.match(line)
+            if match is None:
+                error = 'Invalid LOAD line'
+            else:
+                flags, = match.groups()
+                if 'W' in flags and 'E' in flags:
+                    if xfail:
+                        print('{}: warning: WX segment (as expected)'.format(
+                            path))
+                    else:
+                        error = 'WX segment'
+
+        if error is not None:
+            print('{}: error: {}: {!r}'.format(path, error, line.strip()))
+            errors += 1
+
+    if xfail and errors == 0:
+        print('{}: warning: missing expected WX segment'.format(path))
+    return errors
+
+
+def main():
+    """The main entry point."""
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument('--xfail',
+                        help='Mark input files as XFAILed ("*" for all)',
+                        type=str, default='')
+    parser.add_argument('phdrs',
+                        help='Files containing readelf -Wl output',
+                        nargs='*')
+    opts = parser.parse_args(sys.argv)
+
+    xfails = set(opts.xfail.split(' '))
+    xfails_all = opts.xfail.strip() == '*'
+
+    errors = 0
+    for path in opts.phdrs:
+        xfail = ((os.path.basename(path) + '.phdrs') in xfails
+                 or xfails_all)
+        with open(path) as inp:
+            errors += process_file(path, inp, xfail)
+    if errors > 0:
+        sys.exit(1)
+
+
+if __name__ == '__main__':
+    main()
diff --git a/sysdeps/sparc/Makefile b/sysdeps/sparc/Makefile
index 38b33af6e0..c123b527b5 100644
--- a/sysdeps/sparc/Makefile
+++ b/sysdeps/sparc/Makefile
@@ -16,6 +16,15 @@  CPPFLAGS-crti.S += -fPIC
 CPPFLAGS-crtn.S += -fPIC
 endif
 
+ifeq ($(subdir),elf)
+
+# Lazy binding on SPARC rewrites the PLT sequence.  See the Solaris
+# Linker and Libraries Guide, section SPARC: Procedure Linkage Table.
+# <https://docs.oracle.com/cd/E19455-01/816-0559/chapter6-1236/index.html>
+test-xfail-check-wx-segment = *
+
+endif # $(subdir) == elf
+
 # nscd uses atomic_spin_nop which in turn requires cpu_relax
 ifeq ($(subdir),nscd)
 routines += cpu_relax
diff --git a/sysdeps/unix/sysv/linux/hppa/Makefile b/sysdeps/unix/sysv/linux/hppa/Makefile
index e1637f54f5..c89ec83182 100644
--- a/sysdeps/unix/sysv/linux/hppa/Makefile
+++ b/sysdeps/unix/sysv/linux/hppa/Makefile
@@ -3,9 +3,14 @@  ifeq ($(subdir),stdlib)
 gen-as-const-headers += ucontext_i.sym
 endif
 
+ifeq ($(subdir),elf)
 # Supporting non-executable stacks on HPPA requires changes to both
 # the Linux kernel and glibc. The kernel currently needs an executable
 # stack for syscall restarts and signal returns.
-ifeq ($(subdir),elf)
 test-xfail-check-execstack = yes
-endif
+
+# On hppa, the PLT is executable because it contains an executable
+# trampoline used during lazy binding.
+test-xfail-check-wx-segment = *
+
+endif # $(subdir) == elf