public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v7 0/4] Properly align all PT_LOAD segments with tests
@ 2022-01-03 23:04 H.J. Lu
  2022-01-03 23:04 ` [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676] H.J. Lu
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: H.J. Lu @ 2022-01-03 23:04 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella

Changes from v6:

1. Rebase.
2. Add tests for other p_align cases.

Linker may set p_align of a PT_LOAD segment larger than p_align of the
first PT_LOAD segment to satisfy a section alignment.  We should align
the first PT_LOAD segment to the maximum p_align of all PT_LOAD segments.

H.J. Lu (4):
  elf: Properly align all PT_LOAD segments [BZ #28676]
  elf: Add a test for PT_LOAD segments with mixed p_align [BZ #28676]
  elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688]
  elf: Add a test for PT_LOAD segments with invalid p_align [BZ #28688]

 elf/Makefile               |  42 +++++++++++++
 elf/dl-load.c              |   9 ++-
 elf/tst-elf-edit.h         | 126 +++++++++++++++++++++++++++++++++++++
 elf/tst-p_align1.c         |  27 ++++++++
 elf/tst-p_align2.c         |  27 ++++++++
 elf/tst-p_align3.c         |  27 ++++++++
 elf/tst-p_align3.sh        |  26 ++++++++
 elf/tst-p_alignmod1-edit.c |  34 ++++++++++
 elf/tst-p_alignmod2-edit.c |  27 ++++++++
 elf/tst-p_alignmod3.c      |  22 +++++++
 10 files changed, 366 insertions(+), 1 deletion(-)
 create mode 100644 elf/tst-elf-edit.h
 create mode 100644 elf/tst-p_align1.c
 create mode 100644 elf/tst-p_align2.c
 create mode 100644 elf/tst-p_align3.c
 create mode 100755 elf/tst-p_align3.sh
 create mode 100644 elf/tst-p_alignmod1-edit.c
 create mode 100644 elf/tst-p_alignmod2-edit.c
 create mode 100644 elf/tst-p_alignmod3.c

-- 
2.33.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676]
  2022-01-03 23:04 [PATCH v7 0/4] Properly align all PT_LOAD segments with tests H.J. Lu
@ 2022-01-03 23:04 ` H.J. Lu
  2022-01-18 17:48   ` Adhemerval Zanella
  2022-01-03 23:04 ` [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align " H.J. Lu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-01-03 23:04 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella

Linker may set p_align of a PT_LOAD segment larger than p_align of the
first PT_LOAD segment to satisfy a section alignment:

Elf file type is DYN (Shared object file)
Entry point 0x0
There are 10 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000834 0x0000000000000834  R E    0x1000
  LOAD           0x0000000000000e00 0x0000000000001e00 0x0000000000001e00
                 0x0000000000000230 0x0000000000000230  RW     0x1000
  LOAD           0x0000000000400000 0x0000000000400000 0x0000000000400000
                 0x0000000000000004 0x0000000000000008  RW     0x400000
...

 Section to Segment mapping:
  Segment Sections...
   00     .note.gnu.property .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
   01     .init_array .fini_array .data.rel.ro .dynamic .got .got.plt
   02     .data .bss

We should align the first PT_LOAD segment to the maximum p_align of all
PT_LOAD segments, similar to the kernel commit:

commit ce81bb256a224259ab686742a6284930cbe4f1fa
Author: Chris Kennelly <ckennelly@google.com>
Date:   Thu Oct 15 20:12:32 2020 -0700

    fs/binfmt_elf: use PT_LOAD p_align values for suitable start address
---
 elf/dl-load.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/elf/dl-load.c b/elf/dl-load.c
index ddc4295ef5..109bed3fb5 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1101,6 +1101,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
     size_t nloadcmds = 0;
     bool has_holes = false;
     bool empty_dynamic = false;
+    ElfW(Addr) p_align_max = 0;
 
     /* The struct is initialized to zero so this is not necessary:
     l->l_ld = 0;
@@ -1146,7 +1147,9 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	  c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
 	  c->dataend = ph->p_vaddr + ph->p_filesz;
 	  c->allocend = ph->p_vaddr + ph->p_memsz;
-	  c->mapalign = ph->p_align;
+	  /* Remember the maximum p_align.  */
+	  if (ph->p_align > p_align_max)
+	    p_align_max = ph->p_align;
 	  c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize));
 
 	  /* Determine whether there is a gap between the last segment
@@ -1221,6 +1224,10 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 	goto lose;
       }
 
+    /* Align all PT_LOAD segments to the maximum p_align.  */
+    for (size_t i = 0; i < nloadcmds; i++)
+      loadcmds[i].mapalign = p_align_max;
+
     /* dlopen of an executable is not valid because it is not possible
        to perform proper relocations, handle static TLS, or run the
        ELF constructors.  For PIE, the check needs the dynamic
-- 
2.33.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align [BZ #28676]
  2022-01-03 23:04 [PATCH v7 0/4] Properly align all PT_LOAD segments with tests H.J. Lu
  2022-01-03 23:04 ` [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676] H.J. Lu
@ 2022-01-03 23:04 ` H.J. Lu
  2022-01-18 16:25   ` Adhemerval Zanella
  2022-01-03 23:04 ` [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688] H.J. Lu
  2022-01-03 23:04 ` [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align " H.J. Lu
  3 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-01-03 23:04 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella

Add tst-p_alignmod1-edit to edit the copy of tst-alignmod3.so to reduce
p_align of the first PT_LOAD segment by half and verify that the shared
library is mapped with the maximum p_align of all PT_LOAD segments.
---
 elf/Makefile               |  18 ++++++
 elf/tst-elf-edit.h         | 126 +++++++++++++++++++++++++++++++++++++
 elf/tst-p_align1.c         |  27 ++++++++
 elf/tst-p_alignmod1-edit.c |  34 ++++++++++
 4 files changed, 205 insertions(+)
 create mode 100644 elf/tst-elf-edit.h
 create mode 100644 elf/tst-p_align1.c
 create mode 100644 elf/tst-p_alignmod1-edit.c

diff --git a/elf/Makefile b/elf/Makefile
index 883578ee36..9c831faac1 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -403,6 +403,13 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-dl_find_object-mod8 \
 		tst-dl_find_object-mod9 \
 
+ifeq (yes,$(build-shared))
+tests += \
+  tst-p_alignmod1-edit \
+  tst-p_align1 \
+
+endif
+
 # Most modules build with _ISOMAC defined, but those filtered out
 # depend on internal headers.
 modules-names-tests = $(filter-out ifuncmod% tst-tlsmod%,\
@@ -2079,3 +2086,14 @@ CFLAGS-tst-dl_find_object-mod6.c += -funwind-tables
 CFLAGS-tst-dl_find_object-mod7.c += -funwind-tables
 CFLAGS-tst-dl_find_object-mod8.c += -funwind-tables
 CFLAGS-tst-dl_find_object-mod9.c += -funwind-tables
+
+$(objpfx)tst-p_align1: $(objpfx)tst-p_alignmod1.so
+
+# Make a copy of tst-alignmod3.so and lower p_align of the first PT_LOAD
+# segment.
+$(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
+			     $(objpfx)tst-alignmod3.so
+	rm -f $@
+	cp $(objpfx)tst-alignmod3.so $@
+	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
+	  $(objpfx)tst-p_alignmod1-edit $@
diff --git a/elf/tst-elf-edit.h b/elf/tst-elf-edit.h
new file mode 100644
index 0000000000..ce98cf012c
--- /dev/null
+++ b/elf/tst-elf-edit.h
@@ -0,0 +1,126 @@
+/* Update p_align of the first PT_LOAD segment.
+   Copyright (C) 2021 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/>.  */
+
+#include <stdio.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <link.h>
+#include <error.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+
+const char *file_name;
+
+static size_t update_p_align (size_t);
+
+int
+main (int argc, char ** argv)
+{
+  if (argc != 2)
+    {
+      printf ("Usage: %s: file\n", argv[0]);
+      return 0;
+    }
+
+  file_name = argv[1];
+  struct stat statbuf;
+  int errno_saved;
+
+  if (stat (file_name, &statbuf) < 0)
+    error (1, errno, "%s: not exist", file_name);
+
+  ElfW(Ehdr) *ehdr;
+
+  if (statbuf.st_size < sizeof (*ehdr))
+    error (1, 0, "%s: too small", file_name);
+
+  int fd = open (file_name, O_RDWR);
+  if (fd < 0)
+    error (1, errno, "%s: can't open", file_name);
+
+  /* Map in the whole file.  */
+  void *base = mmap (NULL, statbuf.st_size, PROT_READ | PROT_WRITE,
+		     MAP_SHARED, fd, 0);
+  if (base == MAP_FAILED)
+    {
+      errno_saved = errno;
+      close (fd);
+      error (1, errno_saved, "%s: failed to map", file_name);
+    }
+
+  ehdr = (ElfW(Ehdr) *) base;
+  if (ehdr->e_ident[EI_MAG0] != ELFMAG0
+      || ehdr->e_ident[EI_MAG1] != ELFMAG1
+      || ehdr->e_ident[EI_MAG2] != ELFMAG2
+      || ehdr->e_ident[EI_MAG3] != ELFMAG3)
+    {
+      close (fd);
+      error (1, 0, "%s: bad ELF header", file_name);
+    }
+
+  if (ehdr->e_type != ET_DYN)
+    {
+      close (fd);
+      error (1, 0, "%s: not shared library", file_name);
+    }
+
+  bool unsupported_class = true;
+  switch (ehdr->e_ident[EI_CLASS])
+    {
+    default:
+      break;
+
+    case ELFCLASS32:
+      unsupported_class = __ELF_NATIVE_CLASS != 32;
+      break;
+
+    case ELFCLASS64:
+      unsupported_class = __ELF_NATIVE_CLASS != 64;
+      break;
+    }
+
+  if (unsupported_class)
+    {
+      close (fd);
+      error (1, 0, "%s: unsupported ELF class: %d",
+	     file_name, ehdr->e_ident[EI_CLASS]);
+    }
+
+  size_t phdr_size = sizeof (ElfW(Phdr)) * ehdr->e_phentsize;
+  if (statbuf.st_size < (ehdr->e_phoff + phdr_size))
+    {
+      close (fd);
+      error (1, 0, "%s: too small", file_name);
+    }
+
+  ElfW(Phdr) *phdr = (ElfW(Phdr) *) (base + ehdr->e_phoff);
+  for (int i = 0; i < ehdr->e_phnum; i++, phdr++)
+    if (phdr->p_type == PT_LOAD)
+      {
+	/* Update p_align of the first PT_LOAD segment.  */
+	phdr->p_align = update_p_align (phdr->p_align);
+	break;
+      }
+
+  munmap (base, statbuf.st_size);
+  close (fd);
+
+  return 0;
+}
diff --git a/elf/tst-p_align1.c b/elf/tst-p_align1.c
new file mode 100644
index 0000000000..cab9793220
--- /dev/null
+++ b/elf/tst-p_align1.c
@@ -0,0 +1,27 @@
+/* Check different alignments of PT_LOAD segments in a shared library.
+   Copyright (C) 2021 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/>.  */
+
+extern int do_load_test (void);
+
+static int
+do_test (void)
+{
+  return do_load_test ();
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-p_alignmod1-edit.c b/elf/tst-p_alignmod1-edit.c
new file mode 100644
index 0000000000..2aa0e7ccdd
--- /dev/null
+++ b/elf/tst-p_alignmod1-edit.c
@@ -0,0 +1,34 @@
+/* Reduce p_align of the first PT_LOAD segment by half.
+   Copyright (C) 2021 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/>.  */
+
+#include "tst-elf-edit.h"
+
+/* Reduce p_align by half.  */
+
+static size_t
+update_p_align (size_t p_align)
+{
+  size_t pagesize = sysconf (_SC_PAGESIZE);
+  size_t new_p_align = p_align >> 1;
+
+  if (new_p_align < pagesize)
+    error (1, 0, "%s: new p_align (0x%zx) < page size (0x%zx)",
+	   file_name, new_p_align, pagesize);
+
+  return new_p_align;
+}
-- 
2.33.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688]
  2022-01-03 23:04 [PATCH v7 0/4] Properly align all PT_LOAD segments with tests H.J. Lu
  2022-01-03 23:04 ` [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676] H.J. Lu
  2022-01-03 23:04 ` [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align " H.J. Lu
@ 2022-01-03 23:04 ` H.J. Lu
  2022-01-18 16:32   ` Adhemerval Zanella
  2022-01-03 23:04 ` [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align " H.J. Lu
  3 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-01-03 23:04 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella

Add tst-p_alignmod2-edit to edit the copy of tst-alignmod3.so to set
p_align of the first PT_LOAD segment to 1 and verify that the shared
library can be loaded normally.
---
 elf/Makefile               | 13 +++++++++++++
 elf/tst-p_align2.c         | 27 +++++++++++++++++++++++++++
 elf/tst-p_alignmod2-edit.c | 27 +++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 elf/tst-p_align2.c
 create mode 100644 elf/tst-p_alignmod2-edit.c

diff --git a/elf/Makefile b/elf/Makefile
index 9c831faac1..e4be3fa518 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -406,7 +406,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 ifeq (yes,$(build-shared))
 tests += \
   tst-p_alignmod1-edit \
+  tst-p_alignmod2-edit \
   tst-p_align1 \
+  tst-p_align2 \
 
 endif
 
@@ -2097,3 +2099,14 @@ $(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
 	cp $(objpfx)tst-alignmod3.so $@
 	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
 	  $(objpfx)tst-p_alignmod1-edit $@
+
+$(objpfx)tst-p_align2: $(objpfx)tst-p_alignmod2.so
+
+# Make a copy of tst-alignmod3.so and update p_align of the first PT_LOAD
+# segment.
+$(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
+			     $(objpfx)tst-alignmod3.so
+	rm -f $@
+	cp $(objpfx)tst-alignmod3.so $@
+	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
+	  $(objpfx)tst-p_alignmod2-edit $@
diff --git a/elf/tst-p_align2.c b/elf/tst-p_align2.c
new file mode 100644
index 0000000000..cab9793220
--- /dev/null
+++ b/elf/tst-p_align2.c
@@ -0,0 +1,27 @@
+/* Check different alignments of PT_LOAD segments in a shared library.
+   Copyright (C) 2021 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/>.  */
+
+extern int do_load_test (void);
+
+static int
+do_test (void)
+{
+  return do_load_test ();
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-p_alignmod2-edit.c b/elf/tst-p_alignmod2-edit.c
new file mode 100644
index 0000000000..2f47b37c9e
--- /dev/null
+++ b/elf/tst-p_alignmod2-edit.c
@@ -0,0 +1,27 @@
+/* Set p_align of the first PT_LOAD segment to 1.
+   Copyright (C) 2021 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/>.  */
+
+#include "tst-elf-edit.h"
+
+/* Set p_align to 1.  */
+
+static size_t
+update_p_align (size_t p_align __attribute__ ((unused)))
+{
+  return 1;
+}
-- 
2.33.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align [BZ #28688]
  2022-01-03 23:04 [PATCH v7 0/4] Properly align all PT_LOAD segments with tests H.J. Lu
                   ` (2 preceding siblings ...)
  2022-01-03 23:04 ` [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688] H.J. Lu
@ 2022-01-03 23:04 ` H.J. Lu
  2022-01-18 18:05   ` Adhemerval Zanella
  3 siblings, 1 reply; 14+ messages in thread
From: H.J. Lu @ 2022-01-03 23:04 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer, Adhemerval Zanella

Build tst-p_alignmod3.so with 256 byte page size and verify that it is
rejected with a proper error message.
---
 elf/Makefile          | 11 +++++++++++
 elf/tst-p_align3.c    | 27 +++++++++++++++++++++++++++
 elf/tst-p_align3.sh   | 26 ++++++++++++++++++++++++++
 elf/tst-p_alignmod3.c | 22 ++++++++++++++++++++++
 4 files changed, 86 insertions(+)
 create mode 100644 elf/tst-p_align3.c
 create mode 100755 elf/tst-p_align3.sh
 create mode 100644 elf/tst-p_alignmod3.c

diff --git a/elf/Makefile b/elf/Makefile
index e4be3fa518..eca730ae8f 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -409,6 +409,10 @@ tests += \
   tst-p_alignmod2-edit \
   tst-p_align1 \
   tst-p_align2 \
+  tst-p_align3 \
+
+modules-names += \
+  tst-p_alignmod3 \
 
 endif
 
@@ -2110,3 +2114,10 @@ $(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
 	cp $(objpfx)tst-alignmod3.so $@
 	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
 	  $(objpfx)tst-p_alignmod2-edit $@
+
+LDFLAGS-tst-p_alignmod3.so += -Wl,-z,max-page-size=0x100,-z,common-page-size=0x100
+
+$(objpfx)tst-p_align3: $(objpfx)tst-p_alignmod3.so
+$(objpfx)tst-p_align3.out: tst-p_align3.sh $(objpfx)tst-p_align3
+	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
+	$(evaluate-test)
diff --git a/elf/tst-p_align3.c b/elf/tst-p_align3.c
new file mode 100644
index 0000000000..61c616ac3b
--- /dev/null
+++ b/elf/tst-p_align3.c
@@ -0,0 +1,27 @@
+/* Check invalid p_align of PT_LOAD segments in a shared library.
+   Copyright (C) 2021 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/>.  */
+
+extern int do_load_test (void);
+
+static int
+do_test (void)
+{
+  return do_load_test ();
+}
+
+#include <support/test-driver.c>
diff --git a/elf/tst-p_align3.sh b/elf/tst-p_align3.sh
new file mode 100755
index 0000000000..11572aa217
--- /dev/null
+++ b/elf/tst-p_align3.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+# Check invalid p_align of PT_LOAD segments in a shared library.
+# Copyright (C) 2021 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/>.
+
+common_objpfx=$1; shift
+test_program_prefix=$1; shift
+
+${test_program_prefix} \
+  ${common_objpfx}elf/tst-p_align3 \
+    2> ${common_objpfx}elf/tst-p_align3.out && exit 1
+grep "ELF load command address/offset not page-aligned" ${common_objpfx}elf/tst-p_align3.out
diff --git a/elf/tst-p_alignmod3.c b/elf/tst-p_alignmod3.c
new file mode 100644
index 0000000000..90f3af467e
--- /dev/null
+++ b/elf/tst-p_alignmod3.c
@@ -0,0 +1,22 @@
+/* Check invalid p_align of PT_LOAD segment in a shared library.
+   Copyright (C) 2021 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/>.  */
+
+void
+do_load_test (void)
+{
+}
-- 
2.33.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align [BZ #28676]
  2022-01-03 23:04 ` [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align " H.J. Lu
@ 2022-01-18 16:25   ` Adhemerval Zanella
  2022-01-18 21:53     ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2022-01-18 16:25 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Florian Weimer



On 03/01/2022 20:04, H.J. Lu wrote:
> Add tst-p_alignmod1-edit to edit the copy of tst-alignmod3.so to reduce
> p_align of the first PT_LOAD segment by half and verify that the shared
> library is mapped with the maximum p_align of all PT_LOAD segments.
> ---
>  elf/Makefile               |  18 ++++++
>  elf/tst-elf-edit.h         | 126 +++++++++++++++++++++++++++++++++++++
>  elf/tst-p_align1.c         |  27 ++++++++
>  elf/tst-p_alignmod1-edit.c |  34 ++++++++++
>  4 files changed, 205 insertions(+)
>  create mode 100644 elf/tst-elf-edit.h
>  create mode 100644 elf/tst-p_align1.c
>  create mode 100644 elf/tst-p_alignmod1-edit.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 883578ee36..9c831faac1 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -403,6 +403,13 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>  		tst-dl_find_object-mod8 \
>  		tst-dl_find_object-mod9 \
>  
> +ifeq (yes,$(build-shared))
> +tests += \
> +  tst-p_alignmod1-edit \
> +  tst-p_align1 \
> +
> +endif
> +
>  # Most modules build with _ISOMAC defined, but those filtered out
>  # depend on internal headers.
>  modules-names-tests = $(filter-out ifuncmod% tst-tlsmod%,\
> @@ -2079,3 +2086,14 @@ CFLAGS-tst-dl_find_object-mod6.c += -funwind-tables
>  CFLAGS-tst-dl_find_object-mod7.c += -funwind-tables
>  CFLAGS-tst-dl_find_object-mod8.c += -funwind-tables
>  CFLAGS-tst-dl_find_object-mod9.c += -funwind-tables
> +
> +$(objpfx)tst-p_align1: $(objpfx)tst-p_alignmod1.so
> +
> +# Make a copy of tst-alignmod3.so and lower p_align of the first PT_LOAD
> +# segment.
> +$(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
> +			     $(objpfx)tst-alignmod3.so

tst-alignmod3.so is only provided in the last patch, I think it would be better
to move to this patch as tst-alignmod-base and it instead by copying and changing
on each test.

> +	rm -f $@
> +	cp $(objpfx)tst-alignmod3.so $@
> +	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> +	  $(objpfx)tst-p_alignmod1-edit $@
> diff --git a/elf/tst-elf-edit.h b/elf/tst-elf-edit.h
> new file mode 100644
> index 0000000000..ce98cf012c
> --- /dev/null
> +++ b/elf/tst-elf-edit.h
> @@ -0,0 +1,126 @@
> +/* Update p_align of the first PT_LOAD segment.
> +   Copyright (C) 2021 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/>.  */
> +
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <link.h>
> +#include <error.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +
> +const char *file_name;
> +
> +static size_t update_p_align (size_t);
> +
> +int
> +main (int argc, char ** argv)
> +{
> +  if (argc != 2)
> +    {
> +      printf ("Usage: %s: file\n", argv[0]);
> +      return 0;
> +    }
> +
> +  file_name = argv[1];
> +  struct stat statbuf;
> +  int errno_saved;
> +
> +  if (stat (file_name, &statbuf) < 0)
> +    error (1, errno, "%s: not exist", file_name);
> +
> +  ElfW(Ehdr) *ehdr;
> +
> +  if (statbuf.st_size < sizeof (*ehdr))
> +    error (1, 0, "%s: too small", file_name);
> +
> +  int fd = open (file_name, O_RDWR);
> +  if (fd < 0)
> +    error (1, errno, "%s: can't open", file_name);
> +
> +  /* Map in the whole file.  */
> +  void *base = mmap (NULL, statbuf.st_size, PROT_READ | PROT_WRITE,
> +		     MAP_SHARED, fd, 0);
> +  if (base == MAP_FAILED)
> +    {
> +      errno_saved = errno;
> +      close (fd);
> +      error (1, errno_saved, "%s: failed to map", file_name);
> +    }
> +
> +  ehdr = (ElfW(Ehdr) *) base;
> +  if (ehdr->e_ident[EI_MAG0] != ELFMAG0
> +      || ehdr->e_ident[EI_MAG1] != ELFMAG1
> +      || ehdr->e_ident[EI_MAG2] != ELFMAG2
> +      || ehdr->e_ident[EI_MAG3] != ELFMAG3)
> +    {
> +      close (fd);
> +      error (1, 0, "%s: bad ELF header", file_name);
> +    }
> +
> +  if (ehdr->e_type != ET_DYN)
> +    {
> +      close (fd);
> +      error (1, 0, "%s: not shared library", file_name);
> +    }
> +
> +  bool unsupported_class = true;
> +  switch (ehdr->e_ident[EI_CLASS])
> +    {
> +    default:
> +      break;
> +
> +    case ELFCLASS32:
> +      unsupported_class = __ELF_NATIVE_CLASS != 32;
> +      break;
> +
> +    case ELFCLASS64:
> +      unsupported_class = __ELF_NATIVE_CLASS != 64;
> +      break;
> +    }
> +
> +  if (unsupported_class)
> +    {
> +      close (fd);
> +      error (1, 0, "%s: unsupported ELF class: %d",
> +	     file_name, ehdr->e_ident[EI_CLASS]);
> +    }
> +
> +  size_t phdr_size = sizeof (ElfW(Phdr)) * ehdr->e_phentsize;
> +  if (statbuf.st_size < (ehdr->e_phoff + phdr_size))
> +    {
> +      close (fd);
> +      error (1, 0, "%s: too small", file_name);
> +    }
> +
> +  ElfW(Phdr) *phdr = (ElfW(Phdr) *) (base + ehdr->e_phoff);
> +  for (int i = 0; i < ehdr->e_phnum; i++, phdr++)
> +    if (phdr->p_type == PT_LOAD)
> +      {
> +	/* Update p_align of the first PT_LOAD segment.  */
> +	phdr->p_align = update_p_align (phdr->p_align);
> +	break;
> +      }
> +
> +  munmap (base, statbuf.st_size);
> +  close (fd);
> +
> +  return 0;
> +}

Look ok, although I think it would be simpler to build it once and use an
argument of how to change the alignment instead of build once binary
per test.  I am not if it is worth to use libsupport here.

> diff --git a/elf/tst-p_align1.c b/elf/tst-p_align1.c
> new file mode 100644
> index 0000000000..cab9793220
> --- /dev/null
> +++ b/elf/tst-p_align1.c
> @@ -0,0 +1,27 @@
> +/* Check different alignments of PT_LOAD segments in a shared library.
> +   Copyright (C) 2021 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/>.  */
> +
> +extern int do_load_test (void);

The do_load_test is defined as returning void on elf/tst-alignmod3.c,
but it redefined here as returning 'int'.  This is causing a failure 
on aarch64.

Maybe it would better to move it to a proper header.

> +
> +static int
> +do_test (void)
> +{
> +  return do_load_test ();
> +}
> +
> +#include <support/test-driver.c>

And I think 'is_aligned' from sysdeps/generic/tst-stack-align.h
seems to have the inverted semantic that makes things confusing 
(elf/tst-alignmod3.c check if it is equal to 0 to actually check
if pointer is aligned).

Maybe it would be better to return bool and 'true' if the pointer
is indeed aligned (similar to what IS_ALIGNED/PTR_IS_ALIGNED
internal macros do).

   bool
   __attribute__ ((weak, noclone, noinline))
   is_aligned (void *p, int align)
   {
      return (((uintptr_t) p) & (align - 1)) == 0;
   }

> diff --git a/elf/tst-p_alignmod1-edit.c b/elf/tst-p_alignmod1-edit.c
> new file mode 100644
> index 0000000000..2aa0e7ccdd
> --- /dev/null
> +++ b/elf/tst-p_alignmod1-edit.c
> @@ -0,0 +1,34 @@
> +/* Reduce p_align of the first PT_LOAD segment by half.
> +   Copyright (C) 2021 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/>.  */
> +
> +#include "tst-elf-edit.h"
> +
> +/* Reduce p_align by half.  */
> +
> +static size_t
> +update_p_align (size_t p_align)
> +{
> +  size_t pagesize = sysconf (_SC_PAGESIZE);
> +  size_t new_p_align = p_align >> 1;
> +
> +  if (new_p_align < pagesize)
> +    error (1, 0, "%s: new p_align (0x%zx) < page size (0x%zx)",
> +	   file_name, new_p_align, pagesize);
> +
> +  return new_p_align;
> +}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688]
  2022-01-03 23:04 ` [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688] H.J. Lu
@ 2022-01-18 16:32   ` Adhemerval Zanella
  2022-01-18 16:36     ` Adhemerval Zanella
  2022-01-18 21:55     ` H.J. Lu
  0 siblings, 2 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2022-01-18 16:32 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Florian Weimer



On 03/01/2022 20:04, H.J. Lu wrote:
> Add tst-p_alignmod2-edit to edit the copy of tst-alignmod3.so to set
> p_align of the first PT_LOAD segment to 1 and verify that the shared
> library can be loaded normally.
> ---
>  elf/Makefile               | 13 +++++++++++++
>  elf/tst-p_align2.c         | 27 +++++++++++++++++++++++++++
>  elf/tst-p_alignmod2-edit.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>  create mode 100644 elf/tst-p_align2.c
>  create mode 100644 elf/tst-p_alignmod2-edit.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 9c831faac1..e4be3fa518 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -406,7 +406,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>  ifeq (yes,$(build-shared))
>  tests += \
>    tst-p_alignmod1-edit \
> +  tst-p_alignmod2-edit \
>    tst-p_align1 \
> +  tst-p_align2 \
>  

Maybe add a #test as final member?

>  endif
>  
> @@ -2097,3 +2099,14 @@ $(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
>  	cp $(objpfx)tst-alignmod3.so $@
>  	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
>  	  $(objpfx)tst-p_alignmod1-edit $@
> +
> +$(objpfx)tst-p_align2: $(objpfx)tst-p_alignmod2.so
> +
> +# Make a copy of tst-alignmod3.so and update p_align of the first PT_LOAD
> +# segment.
> +$(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
> +			     $(objpfx)tst-alignmod3.so

Same as previous patch, it uses tst-alignmod3.so which only defined in next one.

> +	rm -f $@
> +	cp $(objpfx)tst-alignmod3.so $@
> +	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> +	  $(objpfx)tst-p_alignmod2-edit $@
> diff --git a/elf/tst-p_align2.c b/elf/tst-p_align2.c
> new file mode 100644
> index 0000000000..cab9793220
> --- /dev/null
> +++ b/elf/tst-p_align2.c
> @@ -0,0 +1,27 @@
> +/* Check different alignments of PT_LOAD segments in a shared library.
> +   Copyright (C) 2021 Free Software Foundation, Inc.

Update the Copyright year.

> +   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/>.  */
> +
> +extern int do_load_test (void);
> +
> +static int
> +do_test (void)
> +{
> +  return do_load_test ();
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-p_alignmod2-edit.c b/elf/tst-p_alignmod2-edit.c
> new file mode 100644
> index 0000000000..2f47b37c9e
> --- /dev/null
> +++ b/elf/tst-p_alignmod2-edit.c
> @@ -0,0 +1,27 @@
> +/* Set p_align of the first PT_LOAD segment to 1.
> +   Copyright (C) 2021 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/>.  */
> +
> +#include "tst-elf-edit.h"
> +
> +/* Set p_align to 1.  */
> +
> +static size_t
> +update_p_align (size_t p_align __attribute__ ((unused)))
> +{
> +  return 1;
> +}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688]
  2022-01-18 16:32   ` Adhemerval Zanella
@ 2022-01-18 16:36     ` Adhemerval Zanella
  2022-01-18 21:55     ` H.J. Lu
  1 sibling, 0 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2022-01-18 16:36 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Florian Weimer



On 18/01/2022 13:32, Adhemerval Zanella wrote:
> 
> 
> On 03/01/2022 20:04, H.J. Lu wrote:
>> Add tst-p_alignmod2-edit to edit the copy of tst-alignmod3.so to set
>> p_align of the first PT_LOAD segment to 1 and verify that the shared
>> library can be loaded normally.
>> ---
>>  elf/Makefile               | 13 +++++++++++++
>>  elf/tst-p_align2.c         | 27 +++++++++++++++++++++++++++
>>  elf/tst-p_alignmod2-edit.c | 27 +++++++++++++++++++++++++++
>>  3 files changed, 67 insertions(+)
>>  create mode 100644 elf/tst-p_align2.c
>>  create mode 100644 elf/tst-p_alignmod2-edit.c
>>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 9c831faac1..e4be3fa518 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -406,7 +406,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
>>  ifeq (yes,$(build-shared))
>>  tests += \
>>    tst-p_alignmod1-edit \
>> +  tst-p_alignmod2-edit \
>>    tst-p_align1 \
>> +  tst-p_align2 \
>>  
> 
> Maybe add a #test as final member?
> 
>>  endif
>>  
>> @@ -2097,3 +2099,14 @@ $(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
>>  	cp $(objpfx)tst-alignmod3.so $@
>>  	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
>>  	  $(objpfx)tst-p_alignmod1-edit $@
>> +
>> +$(objpfx)tst-p_align2: $(objpfx)tst-p_alignmod2.so
>> +
>> +# Make a copy of tst-alignmod3.so and update p_align of the first PT_LOAD
>> +# segment.
>> +$(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
>> +			     $(objpfx)tst-alignmod3.so
> 
> Same as previous patch, it uses tst-alignmod3.so which only defined in next one.
> 
>> +	rm -f $@
>> +	cp $(objpfx)tst-alignmod3.so $@
>> +	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
>> +	  $(objpfx)tst-p_alignmod2-edit $@
>> diff --git a/elf/tst-p_align2.c b/elf/tst-p_align2.c
>> new file mode 100644
>> index 0000000000..cab9793220
>> --- /dev/null
>> +++ b/elf/tst-p_align2.c
>> @@ -0,0 +1,27 @@
>> +/* Check different alignments of PT_LOAD segments in a shared library.
>> +   Copyright (C) 2021 Free Software Foundation, Inc.
> 
> Update the Copyright year.
> 
>> +   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/>.  */
>> +
>> +extern int do_load_test (void);

Same issue as elf/tst-p_align2.c, where do_load_test is defined as returning
void.

>> +
>> +static int
>> +do_test (void)
>> +{
>> +  return do_load_test ();
>> +}
>> +
>> +#include <support/test-driver.c>
>> diff --git a/elf/tst-p_alignmod2-edit.c b/elf/tst-p_alignmod2-edit.c
>> new file mode 100644
>> index 0000000000..2f47b37c9e
>> --- /dev/null
>> +++ b/elf/tst-p_alignmod2-edit.c
>> @@ -0,0 +1,27 @@
>> +/* Set p_align of the first PT_LOAD segment to 1.
>> +   Copyright (C) 2021 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/>.  */
>> +
>> +#include "tst-elf-edit.h"
>> +
>> +/* Set p_align to 1.  */
>> +
>> +static size_t
>> +update_p_align (size_t p_align __attribute__ ((unused)))
>> +{
>> +  return 1;
>> +}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676]
  2022-01-03 23:04 ` [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676] H.J. Lu
@ 2022-01-18 17:48   ` Adhemerval Zanella
  2022-01-18 21:46     ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2022-01-18 17:48 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Florian Weimer



On 03/01/2022 20:04, H.J. Lu wrote:
> Linker may set p_align of a PT_LOAD segment larger than p_align of the
> first PT_LOAD segment to satisfy a section alignment:
> 
> Elf file type is DYN (Shared object file)
> Entry point 0x0
> There are 10 program headers, starting at offset 64
> 
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
>                  0x0000000000000834 0x0000000000000834  R E    0x1000
>   LOAD           0x0000000000000e00 0x0000000000001e00 0x0000000000001e00
>                  0x0000000000000230 0x0000000000000230  RW     0x1000
>   LOAD           0x0000000000400000 0x0000000000400000 0x0000000000400000
>                  0x0000000000000004 0x0000000000000008  RW     0x400000
> ...
> 
>  Section to Segment mapping:
>   Segment Sections...
>    00     .note.gnu.property .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
>    01     .init_array .fini_array .data.rel.ro .dynamic .got .got.plt
>    02     .data .bss
> 
> We should align the first PT_LOAD segment to the maximum p_align of all
> PT_LOAD segments, similar to the kernel commit:
> 
> commit ce81bb256a224259ab686742a6284930cbe4f1fa
> Author: Chris Kennelly <ckennelly@google.com>
> Date:   Thu Oct 15 20:12:32 2020 -0700
> 
>     fs/binfmt_elf: use PT_LOAD p_align values for suitable start address
> ---
>  elf/dl-load.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index ddc4295ef5..109bed3fb5 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1101,6 +1101,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>      size_t nloadcmds = 0;
>      bool has_holes = false;
>      bool empty_dynamic = false;
> +    ElfW(Addr) p_align_max = 0;
>  
>      /* The struct is initialized to zero so this is not necessary:
>      l->l_ld = 0;
> @@ -1146,7 +1147,9 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>  	  c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
>  	  c->dataend = ph->p_vaddr + ph->p_filesz;
>  	  c->allocend = ph->p_vaddr + ph->p_memsz;
> -	  c->mapalign = ph->p_align;
> +	  /* Remember the maximum p_align.  */
> +	  if (ph->p_align > p_align_max)
> +	    p_align_max = ph->p_align;
>  	  c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize));
>  
>  	  /* Determine whether there is a gap between the last segment

Kernel also skips non-power of two alignment as invalid, should we do the same
to consider the max alignment?

> @@ -1221,6 +1224,10 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>  	goto lose;
>        }
>  
> +    /* Align all PT_LOAD segments to the maximum p_align.  */
> +    for (size_t i = 0; i < nloadcmds; i++)
> +      loadcmds[i].mapalign = p_align_max;
> +
>      /* dlopen of an executable is not valid because it is not possible
>         to perform proper relocations, handle static TLS, or run the
>         ELF constructors.  For PIE, the check needs the dynamic

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align [BZ #28688]
  2022-01-03 23:04 ` [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align " H.J. Lu
@ 2022-01-18 18:05   ` Adhemerval Zanella
  2022-01-18 21:50     ` H.J. Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2022-01-18 18:05 UTC (permalink / raw)
  To: H.J. Lu, libc-alpha; +Cc: Florian Weimer



On 03/01/2022 20:04, H.J. Lu wrote:
> Build tst-p_alignmod3.so with 256 byte page size and verify that it is
> rejected with a proper error message.
> ---
>  elf/Makefile          | 11 +++++++++++
>  elf/tst-p_align3.c    | 27 +++++++++++++++++++++++++++
>  elf/tst-p_align3.sh   | 26 ++++++++++++++++++++++++++
>  elf/tst-p_alignmod3.c | 22 ++++++++++++++++++++++
>  4 files changed, 86 insertions(+)
>  create mode 100644 elf/tst-p_align3.c
>  create mode 100755 elf/tst-p_align3.sh
>  create mode 100644 elf/tst-p_alignmod3.c
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index e4be3fa518..eca730ae8f 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -409,6 +409,10 @@ tests += \
>    tst-p_alignmod2-edit \
>    tst-p_align1 \
>    tst-p_align2 \
> +  tst-p_align3 \
> +
> +modules-names += \
> +  tst-p_alignmod3 \
>  
>  endif
>  
> @@ -2110,3 +2114,10 @@ $(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
>  	cp $(objpfx)tst-alignmod3.so $@
>  	$(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
>  	  $(objpfx)tst-p_alignmod2-edit $@
> +
> +LDFLAGS-tst-p_alignmod3.so += -Wl,-z,max-page-size=0x100,-z,common-page-size=0x100
> +
> +$(objpfx)tst-p_align3: $(objpfx)tst-p_alignmod3.so
> +$(objpfx)tst-p_align3.out: tst-p_align3.sh $(objpfx)tst-p_align3
> +	$(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
> +	$(evaluate-test)
> diff --git a/elf/tst-p_align3.c b/elf/tst-p_align3.c
> new file mode 100644
> index 0000000000..61c616ac3b
> --- /dev/null
> +++ b/elf/tst-p_align3.c
> @@ -0,0 +1,27 @@
> +/* Check invalid p_align of PT_LOAD segments in a shared library.
> +   Copyright (C) 2021 Free Software Foundation, Inc.

Update Copyright years.

> +   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/>.  */
> +
> +extern int do_load_test (void);

As before, do_load_test returns void.

> +
> +static int
> +do_test (void)
> +{
> +  return do_load_test ();
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/elf/tst-p_align3.sh b/elf/tst-p_align3.sh
> new file mode 100755
> index 0000000000..11572aa217
> --- /dev/null
> +++ b/elf/tst-p_align3.sh
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +# Check invalid p_align of PT_LOAD segments in a shared library.
> +# Copyright (C) 2021 Free Software Foundation, Inc.

Update Copyright years.

> +# 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/>.
> +
> +common_objpfx=$1; shift
> +test_program_prefix=$1; shift
> +
> +${test_program_prefix} \
> +  ${common_objpfx}elf/tst-p_align3 \
> +    2> ${common_objpfx}elf/tst-p_align3.out && exit 1

I think we should test for command exit output:

  test $? -ne 127 && exit 1

> +grep "ELF load command address/offset not page-aligned" ${common_objpfx}elf/tst-p_align3.out
> diff --git a/elf/tst-p_alignmod3.c b/elf/tst-p_alignmod3.c
> new file mode 100644
> index 0000000000..90f3af467e
> --- /dev/null
> +++ b/elf/tst-p_alignmod3.c
> @@ -0,0 +1,22 @@
> +/* Check invalid p_align of PT_LOAD segment in a shared library.
> +   Copyright (C) 2021 Free Software Foundation, Inc.


Update Copyright years.

> +   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/>.  */
> +
> +void
> +do_load_test (void)
> +{
> +}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676]
  2022-01-18 17:48   ` Adhemerval Zanella
@ 2022-01-18 21:46     ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2022-01-18 21:46 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Florian Weimer

On Tue, Jan 18, 2022 at 9:49 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 03/01/2022 20:04, H.J. Lu wrote:
> > Linker may set p_align of a PT_LOAD segment larger than p_align of the
> > first PT_LOAD segment to satisfy a section alignment:
> >
> > Elf file type is DYN (Shared object file)
> > Entry point 0x0
> > There are 10 program headers, starting at offset 64
> >
> > Program Headers:
> >   Type           Offset             VirtAddr           PhysAddr
> >                  FileSiz            MemSiz              Flags  Align
> >   LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
> >                  0x0000000000000834 0x0000000000000834  R E    0x1000
> >   LOAD           0x0000000000000e00 0x0000000000001e00 0x0000000000001e00
> >                  0x0000000000000230 0x0000000000000230  RW     0x1000
> >   LOAD           0x0000000000400000 0x0000000000400000 0x0000000000400000
> >                  0x0000000000000004 0x0000000000000008  RW     0x400000
> > ...
> >
> >  Section to Segment mapping:
> >   Segment Sections...
> >    00     .note.gnu.property .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .plt.got .text .fini .rodata .eh_frame_hdr .eh_frame
> >    01     .init_array .fini_array .data.rel.ro .dynamic .got .got.plt
> >    02     .data .bss
> >
> > We should align the first PT_LOAD segment to the maximum p_align of all
> > PT_LOAD segments, similar to the kernel commit:
> >
> > commit ce81bb256a224259ab686742a6284930cbe4f1fa
> > Author: Chris Kennelly <ckennelly@google.com>
> > Date:   Thu Oct 15 20:12:32 2020 -0700
> >
> >     fs/binfmt_elf: use PT_LOAD p_align values for suitable start address
> > ---
> >  elf/dl-load.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/elf/dl-load.c b/elf/dl-load.c
> > index ddc4295ef5..109bed3fb5 100644
> > --- a/elf/dl-load.c
> > +++ b/elf/dl-load.c
> > @@ -1101,6 +1101,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >      size_t nloadcmds = 0;
> >      bool has_holes = false;
> >      bool empty_dynamic = false;
> > +    ElfW(Addr) p_align_max = 0;
> >
> >      /* The struct is initialized to zero so this is not necessary:
> >      l->l_ld = 0;
> > @@ -1146,7 +1147,9 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >         c->mapend = ALIGN_UP (ph->p_vaddr + ph->p_filesz, GLRO(dl_pagesize));
> >         c->dataend = ph->p_vaddr + ph->p_filesz;
> >         c->allocend = ph->p_vaddr + ph->p_memsz;
> > -       c->mapalign = ph->p_align;
> > +       /* Remember the maximum p_align.  */
> > +       if (ph->p_align > p_align_max)
> > +         p_align_max = ph->p_align;
> >         c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize));
> >
> >         /* Determine whether there is a gap between the last segment
>
> Kernel also skips non-power of two alignment as invalid, should we do the same
> to consider the max alignment?

Fixed.

> > @@ -1221,6 +1224,10 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
> >       goto lose;
> >        }
> >
> > +    /* Align all PT_LOAD segments to the maximum p_align.  */
> > +    for (size_t i = 0; i < nloadcmds; i++)
> > +      loadcmds[i].mapalign = p_align_max;
> > +
> >      /* dlopen of an executable is not valid because it is not possible
> >         to perform proper relocations, handle static TLS, or run the
> >         ELF constructors.  For PIE, the check needs the dynamic

Thanks.


-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align [BZ #28688]
  2022-01-18 18:05   ` Adhemerval Zanella
@ 2022-01-18 21:50     ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2022-01-18 21:50 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Florian Weimer

On Tue, Jan 18, 2022 at 10:05 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 03/01/2022 20:04, H.J. Lu wrote:
> > Build tst-p_alignmod3.so with 256 byte page size and verify that it is
> > rejected with a proper error message.
> > ---
> >  elf/Makefile          | 11 +++++++++++
> >  elf/tst-p_align3.c    | 27 +++++++++++++++++++++++++++
> >  elf/tst-p_align3.sh   | 26 ++++++++++++++++++++++++++
> >  elf/tst-p_alignmod3.c | 22 ++++++++++++++++++++++
> >  4 files changed, 86 insertions(+)
> >  create mode 100644 elf/tst-p_align3.c
> >  create mode 100755 elf/tst-p_align3.sh
> >  create mode 100644 elf/tst-p_alignmod3.c
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index e4be3fa518..eca730ae8f 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -409,6 +409,10 @@ tests += \
> >    tst-p_alignmod2-edit \
> >    tst-p_align1 \
> >    tst-p_align2 \
> > +  tst-p_align3 \
> > +
> > +modules-names += \
> > +  tst-p_alignmod3 \
> >
> >  endif
> >
> > @@ -2110,3 +2114,10 @@ $(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
> >       cp $(objpfx)tst-alignmod3.so $@
> >       $(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> >         $(objpfx)tst-p_alignmod2-edit $@
> > +
> > +LDFLAGS-tst-p_alignmod3.so += -Wl,-z,max-page-size=0x100,-z,common-page-size=0x100
> > +
> > +$(objpfx)tst-p_align3: $(objpfx)tst-p_alignmod3.so
> > +$(objpfx)tst-p_align3.out: tst-p_align3.sh $(objpfx)tst-p_align3
> > +     $(SHELL) $< $(common-objpfx) '$(test-program-prefix)'; \
> > +     $(evaluate-test)
> > diff --git a/elf/tst-p_align3.c b/elf/tst-p_align3.c
> > new file mode 100644
> > index 0000000000..61c616ac3b
> > --- /dev/null
> > +++ b/elf/tst-p_align3.c
> > @@ -0,0 +1,27 @@
> > +/* Check invalid p_align of PT_LOAD segments in a shared library.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
>
> Update Copyright years.

Fixed.

> > +   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/>.  */
> > +
> > +extern int do_load_test (void);
>
> As before, do_load_test returns void.

Fixed.

> > +
> > +static int
> > +do_test (void)
> > +{
> > +  return do_load_test ();
> > +}
> > +
> > +#include <support/test-driver.c>
> > diff --git a/elf/tst-p_align3.sh b/elf/tst-p_align3.sh
> > new file mode 100755
> > index 0000000000..11572aa217
> > --- /dev/null
> > +++ b/elf/tst-p_align3.sh
> > @@ -0,0 +1,26 @@
> > +#!/bin/sh
> > +# Check invalid p_align of PT_LOAD segments in a shared library.
> > +# Copyright (C) 2021 Free Software Foundation, Inc.
>
> Update Copyright years.

Fixed.

> > +# 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/>.
> > +
> > +common_objpfx=$1; shift
> > +test_program_prefix=$1; shift
> > +
> > +${test_program_prefix} \
> > +  ${common_objpfx}elf/tst-p_align3 \
> > +    2> ${common_objpfx}elf/tst-p_align3.out && exit 1
>
> I think we should test for command exit output:
>
>   test $? -ne 127 && exit 1

Fixed.

> > +grep "ELF load command address/offset not page-aligned" ${common_objpfx}elf/tst-p_align3.out
> > diff --git a/elf/tst-p_alignmod3.c b/elf/tst-p_alignmod3.c
> > new file mode 100644
> > index 0000000000..90f3af467e
> > --- /dev/null
> > +++ b/elf/tst-p_alignmod3.c
> > @@ -0,0 +1,22 @@
> > +/* Check invalid p_align of PT_LOAD segment in a shared library.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
>
>
> Update Copyright years.

Fixed.

> > +   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/>.  */
> > +
> > +void
> > +do_load_test (void)
> > +{
> > +}

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align [BZ #28676]
  2022-01-18 16:25   ` Adhemerval Zanella
@ 2022-01-18 21:53     ` H.J. Lu
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2022-01-18 21:53 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Florian Weimer

On Tue, Jan 18, 2022 at 8:26 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 03/01/2022 20:04, H.J. Lu wrote:
> > Add tst-p_alignmod1-edit to edit the copy of tst-alignmod3.so to reduce
> > p_align of the first PT_LOAD segment by half and verify that the shared
> > library is mapped with the maximum p_align of all PT_LOAD segments.
> > ---
> >  elf/Makefile               |  18 ++++++
> >  elf/tst-elf-edit.h         | 126 +++++++++++++++++++++++++++++++++++++
> >  elf/tst-p_align1.c         |  27 ++++++++
> >  elf/tst-p_alignmod1-edit.c |  34 ++++++++++
> >  4 files changed, 205 insertions(+)
> >  create mode 100644 elf/tst-elf-edit.h
> >  create mode 100644 elf/tst-p_align1.c
> >  create mode 100644 elf/tst-p_alignmod1-edit.c
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 883578ee36..9c831faac1 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -403,6 +403,13 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
> >               tst-dl_find_object-mod8 \
> >               tst-dl_find_object-mod9 \
> >
> > +ifeq (yes,$(build-shared))
> > +tests += \
> > +  tst-p_alignmod1-edit \
> > +  tst-p_align1 \
> > +
> > +endif
> > +
> >  # Most modules build with _ISOMAC defined, but those filtered out
> >  # depend on internal headers.
> >  modules-names-tests = $(filter-out ifuncmod% tst-tlsmod%,\
> > @@ -2079,3 +2086,14 @@ CFLAGS-tst-dl_find_object-mod6.c += -funwind-tables
> >  CFLAGS-tst-dl_find_object-mod7.c += -funwind-tables
> >  CFLAGS-tst-dl_find_object-mod8.c += -funwind-tables
> >  CFLAGS-tst-dl_find_object-mod9.c += -funwind-tables
> > +
> > +$(objpfx)tst-p_align1: $(objpfx)tst-p_alignmod1.so
> > +
> > +# Make a copy of tst-alignmod3.so and lower p_align of the first PT_LOAD
> > +# segment.
> > +$(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
> > +                          $(objpfx)tst-alignmod3.so
>
> tst-alignmod3.so is only provided in the last patch, I think it would be better
> to move to this patch as tst-alignmod-base, not  and it instead by copying and changing
> on each test.

It is tst-alignmod3.so, not tst-p_alignmod3.so.  In any case, I added
tst-p_alignmod-base.so.

> > +     rm -f $@
> > +     cp $(objpfx)tst-alignmod3.so $@
> > +     $(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> > +       $(objpfx)tst-p_alignmod1-edit $@
> > diff --git a/elf/tst-elf-edit.h b/elf/tst-elf-edit.h
> > new file mode 100644
> > index 0000000000..ce98cf012c
> > --- /dev/null
> > +++ b/elf/tst-elf-edit.h
> > @@ -0,0 +1,126 @@
> > +/* Update p_align of the first PT_LOAD segment.
> > +   Copyright (C) 2021 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/>.  */
> > +
> > +#include <stdio.h>
> > +#include <stdbool.h>
> > +#include <unistd.h>
> > +#include <fcntl.h>
> > +#include <link.h>
> > +#include <error.h>
> > +#include <errno.h>
> > +#include <sys/stat.h>
> > +#include <sys/mman.h>
> > +
> > +const char *file_name;
> > +
> > +static size_t update_p_align (size_t);
> > +
> > +int
> > +main (int argc, char ** argv)
> > +{
> > +  if (argc != 2)
> > +    {
> > +      printf ("Usage: %s: file\n", argv[0]);
> > +      return 0;
> > +    }
> > +
> > +  file_name = argv[1];
> > +  struct stat statbuf;
> > +  int errno_saved;
> > +
> > +  if (stat (file_name, &statbuf) < 0)
> > +    error (1, errno, "%s: not exist", file_name);
> > +
> > +  ElfW(Ehdr) *ehdr;
> > +
> > +  if (statbuf.st_size < sizeof (*ehdr))
> > +    error (1, 0, "%s: too small", file_name);
> > +
> > +  int fd = open (file_name, O_RDWR);
> > +  if (fd < 0)
> > +    error (1, errno, "%s: can't open", file_name);
> > +
> > +  /* Map in the whole file.  */
> > +  void *base = mmap (NULL, statbuf.st_size, PROT_READ | PROT_WRITE,
> > +                  MAP_SHARED, fd, 0);
> > +  if (base == MAP_FAILED)
> > +    {
> > +      errno_saved = errno;
> > +      close (fd);
> > +      error (1, errno_saved, "%s: failed to map", file_name);
> > +    }
> > +
> > +  ehdr = (ElfW(Ehdr) *) base;
> > +  if (ehdr->e_ident[EI_MAG0] != ELFMAG0
> > +      || ehdr->e_ident[EI_MAG1] != ELFMAG1
> > +      || ehdr->e_ident[EI_MAG2] != ELFMAG2
> > +      || ehdr->e_ident[EI_MAG3] != ELFMAG3)
> > +    {
> > +      close (fd);
> > +      error (1, 0, "%s: bad ELF header", file_name);
> > +    }
> > +
> > +  if (ehdr->e_type != ET_DYN)
> > +    {
> > +      close (fd);
> > +      error (1, 0, "%s: not shared library", file_name);
> > +    }
> > +
> > +  bool unsupported_class = true;
> > +  switch (ehdr->e_ident[EI_CLASS])
> > +    {
> > +    default:
> > +      break;
> > +
> > +    case ELFCLASS32:
> > +      unsupported_class = __ELF_NATIVE_CLASS != 32;
> > +      break;
> > +
> > +    case ELFCLASS64:
> > +      unsupported_class = __ELF_NATIVE_CLASS != 64;
> > +      break;
> > +    }
> > +
> > +  if (unsupported_class)
> > +    {
> > +      close (fd);
> > +      error (1, 0, "%s: unsupported ELF class: %d",
> > +          file_name, ehdr->e_ident[EI_CLASS]);
> > +    }
> > +
> > +  size_t phdr_size = sizeof (ElfW(Phdr)) * ehdr->e_phentsize;
> > +  if (statbuf.st_size < (ehdr->e_phoff + phdr_size))
> > +    {
> > +      close (fd);
> > +      error (1, 0, "%s: too small", file_name);
> > +    }
> > +
> > +  ElfW(Phdr) *phdr = (ElfW(Phdr) *) (base + ehdr->e_phoff);
> > +  for (int i = 0; i < ehdr->e_phnum; i++, phdr++)
> > +    if (phdr->p_type == PT_LOAD)
> > +      {
> > +     /* Update p_align of the first PT_LOAD segment.  */
> > +     phdr->p_align = update_p_align (phdr->p_align);
> > +     break;
> > +      }
> > +
> > +  munmap (base, statbuf.st_size);
> > +  close (fd);
> > +
> > +  return 0;
> > +}
>
> Look ok, although I think it would be simpler to build it once and use an
> argument of how to change the alignment instead of build once binary
> per test.  I am not if it is worth to use libsupport here.

This can be more flexible than an argument.  I prefer to keep
it ASIS.

> > diff --git a/elf/tst-p_align1.c b/elf/tst-p_align1.c
> > new file mode 100644
> > index 0000000000..cab9793220
> > --- /dev/null
> > +++ b/elf/tst-p_align1.c
> > @@ -0,0 +1,27 @@
> > +/* Check different alignments of PT_LOAD segments in a shared library.
> > +   Copyright (C) 2021 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/>.  */
> > +
> > +extern int do_load_test (void);
>
> The do_load_test is defined as returning void on elf/tst-alignmod3.c,
> but it redefined here as returning 'int'.  This is causing a failure
> on aarch64.
>
> Maybe it would better to move it to a proper header.

Done.

> > +
> > +static int
> > +do_test (void)
> > +{
> > +  return do_load_test ();
> > +}
> > +
> > +#include <support/test-driver.c>
>
> And I think 'is_aligned' from sysdeps/generic/tst-stack-align.h
> seems to have the inverted semantic that makes things confusing
> (elf/tst-alignmod3.c check if it is equal to 0 to actually check
> if pointer is aligned).
>
> Maybe it would be better to return bool and 'true' if the pointer
> is indeed aligned (similar to what IS_ALIGNED/PTR_IS_ALIGNED
> internal macros do).

Fixed.

>    bool
>    __attribute__ ((weak, noclone, noinline))
>    is_aligned (void *p, int align)
>    {
>       return (((uintptr_t) p) & (align - 1)) == 0;
>    }
>
> > diff --git a/elf/tst-p_alignmod1-edit.c b/elf/tst-p_alignmod1-edit.c
> > new file mode 100644
> > index 0000000000..2aa0e7ccdd
> > --- /dev/null
> > +++ b/elf/tst-p_alignmod1-edit.c
> > @@ -0,0 +1,34 @@
> > +/* Reduce p_align of the first PT_LOAD segment by half.
> > +   Copyright (C) 2021 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/>.  */
> > +
> > +#include "tst-elf-edit.h"
> > +
> > +/* Reduce p_align by half.  */
> > +
> > +static size_t
> > +update_p_align (size_t p_align)
> > +{
> > +  size_t pagesize = sysconf (_SC_PAGESIZE);
> > +  size_t new_p_align = p_align >> 1;
> > +
> > +  if (new_p_align < pagesize)
> > +    error (1, 0, "%s: new p_align (0x%zx) < page size (0x%zx)",
> > +        file_name, new_p_align, pagesize);
> > +
> > +  return new_p_align;
> > +}

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688]
  2022-01-18 16:32   ` Adhemerval Zanella
  2022-01-18 16:36     ` Adhemerval Zanella
@ 2022-01-18 21:55     ` H.J. Lu
  1 sibling, 0 replies; 14+ messages in thread
From: H.J. Lu @ 2022-01-18 21:55 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Florian Weimer

On Tue, Jan 18, 2022 at 8:32 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 03/01/2022 20:04, H.J. Lu wrote:
> > Add tst-p_alignmod2-edit to edit the copy of tst-alignmod3.so to set
> > p_align of the first PT_LOAD segment to 1 and verify that the shared
> > library can be loaded normally.
> > ---
> >  elf/Makefile               | 13 +++++++++++++
> >  elf/tst-p_align2.c         | 27 +++++++++++++++++++++++++++
> >  elf/tst-p_alignmod2-edit.c | 27 +++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+)
> >  create mode 100644 elf/tst-p_align2.c
> >  create mode 100644 elf/tst-p_alignmod2-edit.c
> >
> > diff --git a/elf/Makefile b/elf/Makefile
> > index 9c831faac1..e4be3fa518 100644
> > --- a/elf/Makefile
> > +++ b/elf/Makefile
> > @@ -406,7 +406,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
> >  ifeq (yes,$(build-shared))
> >  tests += \
> >    tst-p_alignmod1-edit \
> > +  tst-p_alignmod2-edit \
> >    tst-p_align1 \
> > +  tst-p_align2 \
> >
>
> Maybe add a #test as final member?

Fixed.

> >  endif
> >
> > @@ -2097,3 +2099,14 @@ $(objpfx)tst-p_alignmod1.so: $(objpfx)tst-p_alignmod1-edit \
> >       cp $(objpfx)tst-alignmod3.so $@
> >       $(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> >         $(objpfx)tst-p_alignmod1-edit $@
> > +
> > +$(objpfx)tst-p_align2: $(objpfx)tst-p_alignmod2.so
> > +
> > +# Make a copy of tst-alignmod3.so and update p_align of the first PT_LOAD
> > +# segment.
> > +$(objpfx)tst-p_alignmod2.so: $(objpfx)tst-p_alignmod2-edit \
> > +                          $(objpfx)tst-alignmod3.so
>
> Same as previous patch, it uses tst-alignmod3.so which only defined in next one.

Fixed.

> > +     rm -f $@
> > +     cp $(objpfx)tst-alignmod3.so $@
> > +     $(test-wrapper-env) $(run-program-env) $(rtld-prefix) \
> > +       $(objpfx)tst-p_alignmod2-edit $@
> > diff --git a/elf/tst-p_align2.c b/elf/tst-p_align2.c
> > new file mode 100644
> > index 0000000000..cab9793220
> > --- /dev/null
> > +++ b/elf/tst-p_align2.c
> > @@ -0,0 +1,27 @@
> > +/* Check different alignments of PT_LOAD segments in a shared library.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
>
> Update the Copyright year.

Fixed.

> > +   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/>.  */
> > +
> > +extern int do_load_test (void);
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  return do_load_test ();
> > +}
> > +
> > +#include <support/test-driver.c>
> > diff --git a/elf/tst-p_alignmod2-edit.c b/elf/tst-p_alignmod2-edit.c
> > new file mode 100644
> > index 0000000000..2f47b37c9e
> > --- /dev/null
> > +++ b/elf/tst-p_alignmod2-edit.c
> > @@ -0,0 +1,27 @@
> > +/* Set p_align of the first PT_LOAD segment to 1.
> > +   Copyright (C) 2021 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/>.  */
> > +
> > +#include "tst-elf-edit.h"
> > +
> > +/* Set p_align to 1.  */
> > +
> > +static size_t
> > +update_p_align (size_t p_align __attribute__ ((unused)))
> > +{
> > +  return 1;
> > +}

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-18 21:56 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 23:04 [PATCH v7 0/4] Properly align all PT_LOAD segments with tests H.J. Lu
2022-01-03 23:04 ` [PATCH v7 1/4] elf: Properly align all PT_LOAD segments [BZ #28676] H.J. Lu
2022-01-18 17:48   ` Adhemerval Zanella
2022-01-18 21:46     ` H.J. Lu
2022-01-03 23:04 ` [PATCH v7 2/4] elf: Add a test for PT_LOAD segments with mixed p_align " H.J. Lu
2022-01-18 16:25   ` Adhemerval Zanella
2022-01-18 21:53     ` H.J. Lu
2022-01-03 23:04 ` [PATCH v7 3/4] elf: Add a test for PT_LOAD segments with p_align == 1 [BZ #28688] H.J. Lu
2022-01-18 16:32   ` Adhemerval Zanella
2022-01-18 16:36     ` Adhemerval Zanella
2022-01-18 21:55     ` H.J. Lu
2022-01-03 23:04 ` [PATCH v7 4/4] elf: Add a test for PT_LOAD segments with invalid p_align " H.J. Lu
2022-01-18 18:05   ` Adhemerval Zanella
2022-01-18 21:50     ` H.J. Lu

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).