Re: [PING][PATCH] Segfault while unwinding an invalid function pointer

[Pete Eberlein <eberlein at linux dot vnet dot ibm dot com>]

Andrew Pinksi wrote:
> I don't like the use of the hard coded /tmp/null here.
>
> And I don't like how on open you abort the program instead of just
> doing the same as what happened before.
>
> Also I don't like errno being touched here.

Valid concerns, which I have addressed in a new patch below.  The 
temporary file is aquired using tmpfile().  In the case that the temp 
file could not be opened, the memory is assumed to be ok, and will 
perform as before.  Errno is saved and restored before return.

Andrew Haley wrote:
> I suspect the reason for this is that everybody hates it but nobody
> can think of a better way to do the same thing.  The idea of trapping
> in the kernel via a write(2) call is clever, but it's also very
> heavyweight.  I also agree with all of Pinski's objections.

The heavyweight part (opening a file) will only happen under rare 
circumstances: when fde==NULL (unwind-dw2.c (uw_frame_state_for)) as 
frame unwind info could not be found for that function.  This should 
only happen for a signal frame or a garbage address.

Please review my revised patch.  I thank you both for your feedback.

--
Pete Eberlein
IBM Linux Technology Center
Linux on Power Toolchain


2007-10-29  Pete Eberlein  <eberlein@us.ibm.com>

    * linux-unwind.h (unwind_memory_range_check): New function
    that determines if a memory range is readable.
    * linux-unwind.h (x86_64_fallback_frame_state): Handle
    invalid function pointer address and change cfa.

Index: gcc/config/i386/linux-unwind.h
===================================================================
--- gcc/config/i386/linux-unwind.h      (revision 129738)
+++ gcc/config/i386/linux-unwind.h      (working copy)
@@ -39,6 +39,35 @@

 #define MD_FALLBACK_FRAME_STATE_FOR x86_64_fallback_frame_state

+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+static int
+unwind_memory_range_check (void *addr, size_t len)
+/* returns 0 if memory is unreadable, 1 if ok */
+{
+  size_t rc;
+  int result = 1;
+  FILE* tmp_file;
+  int saved_errno;
+
+  saved_errno = errno;
+  errno = 0;
+
+  tmp_file = tmpfile();
+  if (tmp_file)
+    {
+      rc = write (fileno(tmp_file), addr, len);
+      result = (rc == len);
+      fclose (tmp_file);
+    }
+
+  errno = saved_errno;
+  return result;
+}
+
 static _Unwind_Reason_Code
 x86_64_fallback_frame_state (struct _Unwind_Context *context,
                             _Unwind_FrameState *fs)
@@ -47,6 +76,39 @@
   struct sigcontext *sc;
   long new_cfa;

+  if (!unwind_memory_range_check(pc, 5))
+    {
+      /* memory was invalid */
+
+      fs->regs.cfa_how = CFA_REG_OFFSET;
+      /* Register 7 is rsp  */
+      fs->regs.cfa_reg = 7;
+      fs->regs.cfa_offset = 8;
+
+      fs->regs.reg[0].how = REG_UNSAVED;
+      fs->regs.reg[1].how = REG_UNSAVED;
+      fs->regs.reg[2].how = REG_UNSAVED;
+      fs->regs.reg[3].how = REG_UNSAVED;
+      fs->regs.reg[4].how = REG_UNSAVED;
+      fs->regs.reg[5].how = REG_UNSAVED;
+      fs->regs.reg[6].how = REG_UNSAVED;
+      fs->regs.reg[8].how = REG_UNSAVED;
+      fs->regs.reg[9].how = REG_UNSAVED;
+      fs->regs.reg[10].how = REG_UNSAVED;
+      fs->regs.reg[11].how = REG_UNSAVED;
+      fs->regs.reg[12].how = REG_UNSAVED;
+      fs->regs.reg[13].how = REG_UNSAVED;
+      fs->regs.reg[14].how = REG_UNSAVED;
+      fs->regs.reg[15].how = REG_UNSAVED;
+      fs->regs.reg[16].how = REG_SAVED_OFFSET;
+      fs->regs.reg[16].loc.offset = -8;
+
+      fs->retaddr_column = 16;
+      fs->signal_frame = 0;
+
+      return _URC_NO_REASON;
+    }
+
   /* movq __NR_rt_sigreturn, %rax ; syscall  */
   if (*(unsigned char *)(pc+0) == 0x48
       && *(unsigned long *)(pc+1) == 0x050f0000000fc0c7)