[PATCH] debug: Reject %n format strings in RELRO of static main program (bug 33183)

[Florian Weimer]

Fortification should trap on %n strings in the RELRO area of a
statically linked main program because RELRO may not be in effect
with static linking (whether or not the program header exists).

While at itm fix the return type of check_relro in
elf/dl-readonly-area.c.  It happened to work correctly because
only two enum constants with index 0 and 1 were used before.

---
 debug/Makefile                     |  2 ++
 debug/tst-sprintf-fortify-rdonly.c | 27 +++++++++++++++++++++++----
 elf/dl-readonly-area.c             |  9 ++++++++-
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/debug/Makefile b/debug/Makefile
index 919bf91c42..412917d6ca 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -343,6 +343,8 @@ tests-static = \
   tst-sprintf-fortify-rdonly-static \
   # tests-static
 
+CFLAGS-tst-sprintf-fortify-rdonly-static.c += -DSTATIC_TEST
+
 tests-time64 += \
   $(tests-all-time64-chk) \
   # tests-time64
diff --git a/debug/tst-sprintf-fortify-rdonly.c b/debug/tst-sprintf-fortify-rdonly.c
index fafc8340ea..969af820ed 100644
--- a/debug/tst-sprintf-fortify-rdonly.c
+++ b/debug/tst-sprintf-fortify-rdonly.c
@@ -31,7 +31,8 @@
 
 static sigjmp_buf chk_fail_buf;
 static volatile int ret;
-static bool chk_fail_ok;
+static volatile int chk_fail_line;
+static volatile bool chk_fail_ok;
 
 static void
 handler (int sig)
@@ -42,12 +43,17 @@ handler (int sig)
       longjmp (chk_fail_buf, 1);
     }
   else
-    _exit (127);
+    {
+      printf ("error: fortify failure in block starting on line %d\n",
+	      chk_fail_line);
+      _exit (127);
+    }
 }
 
 #define FORTIFY_FAIL \
   do { printf ("Failure on line %d\n", __LINE__); ret = 1; } while (0)
 #define CHK_FAIL_START \
+  chk_fail_line = __LINE__;			\
   chk_fail_ok = true;				\
   if (! sigsetjmp (chk_fail_buf, 1))		\
     {
@@ -143,23 +149,36 @@ do_test (void)
     CHK_FAIL_END
   }
 
-  /* relro_format is at a readonly part of .bss segment, so '%n' in format input
-     should not trigger a fortify failure.  */
+  /* relro_format is at a readonly part of .bss segment, so '%n' in
+     format input should not trigger a fortify failure.  However, for
+     static builds, RELRO may not be in effect.  */
   {
     char buf[128];
     int n1;
     int n2;
+#ifdef STATIC_TEST
+    CHK_FAIL_START
+#endif
     if (sprintf (buf, relro_format, str2, &n1, str2, &n2) != 2
 	|| n1 != 1 || n2 != 2)
       FAIL_EXIT1 ("sprintf failed: %s %d %d", buf, n1, n2);
+#ifdef STATIC_TEST
+    CHK_FAIL_END
+#endif
   }
 
   /* Same as before, but from an library.  */
   {
     int n1;
     int n2;
+#ifdef STATIC_TEST
+    CHK_FAIL_START
+#endif
     if (sprintf_relro (&n1, &n2) != 2 || n1 != 1 || n2 != 2)
       FAIL_EXIT1 ("sprintf failed: %d %d", n1, n2);
+#ifdef STATIC_TEST
+    CHK_FAIL_END
+#endif
   }
 
   {
diff --git a/elf/dl-readonly-area.c b/elf/dl-readonly-area.c
index 22769ec9d9..471f1cd4bc 100644
--- a/elf/dl-readonly-area.c
+++ b/elf/dl-readonly-area.c
@@ -18,9 +18,16 @@
 
 #include <ldsodefs.h>
 
-static bool
+static enum dl_readonly_area_error_type
 check_relro (const struct link_map *l, uintptr_t start, uintptr_t end)
 {
+#ifndef SHARED
+  /* Statically linked binaries may not use relocation, so RELRO could
+     remain writable.  For consistency, apply this for static PIE, too
+     (where there is relocation processing).  */
+  if (l == GL(dl_ns)[LM_ID_BASE]._ns_loaded)
+    return dl_readonly_area_writable;
+#endif
   if (l->l_relro_addr != 0)
     {
       uintptr_t relro_start = ALIGN_DOWN (l->l_addr + l->l_relro_addr,

base-commit: 1944817240eeea5c044995e45a771a5a5193ecf1