public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] Segfault while unwinding an invalid function pointer
       [not found] ` <4730D908.8090401@linux.vnet.ibm.com>
@ 2007-11-06 21:18   ` Pete Eberlein
  2007-11-16 22:25     ` Pete Eberlein
  0 siblings, 1 reply; 13+ messages in thread
From: Pete Eberlein @ 2007-11-06 21:18 UTC (permalink / raw)
  To: gcc-patches

Thanks to Richard and Ulrich for suggesting mincore.  Thanks to Jakub 
for testing performance.  I've reworked the patch to use mincore below.

Comments?

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


2007-11-06  Pete Eberlein  <eberlein@us.ibm.com>

     * 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 129944)
+++ gcc/config/i386/linux-unwind.h      (working copy)
@@ -39,6 +39,10 @@

  #define MD_FALLBACK_FRAME_STATE_FOR x86_64_fallback_frame_state

+#include <unistd.h>
+#include <sys/mman.h>
+
+
  static _Unwind_Reason_Code
  x86_64_fallback_frame_state (struct _Unwind_Context *context,
                              _Unwind_FrameState *fs)
@@ -46,7 +50,52 @@
    unsigned char *pc = context->ra;
    struct sigcontext *sc;
    long new_cfa;
+  int mem_invalid, saved_errno;
+  char dummy[2];
+  unsigned long start;

+
+  /* check if memory is readable (using mincore) */
+  saved_errno = errno;
+  start = ((unsigned long)pc) & ~(sysconf(_SC_PAGE_SIZE) - 1);
+  mem_invalid = mincore((void*)start, (unsigned long)pc + 9 - start, 
dummy);
+  if (errno != ENOMEM && errno != EINVAL)
+    mem_invalid = 0;
+  errno = saved_errno;
+
+  if (mem_invalid)
+    {
+      /* 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)


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

* Re:[PING] [PATCH] Segfault while unwinding an invalid function pointer
  2007-11-06 21:18   ` [PATCH] Segfault while unwinding an invalid function pointer Pete Eberlein
@ 2007-11-16 22:25     ` Pete Eberlein
  0 siblings, 0 replies; 13+ messages in thread
From: Pete Eberlein @ 2007-11-16 22:25 UTC (permalink / raw)
  To: gcc-patches


http://gcc.gnu.org/ml/gcc-patches/2007-11/msg00281.html

If there are no objections to including this in mainline, could I get an 
ok for Peter Bergner to commit this patch?

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

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

* [PATCH] Segfault while unwinding an invalid function pointer
@ 2008-04-11  4:23 Pete Eberlein
  0 siblings, 0 replies; 13+ messages in thread
From: Pete Eberlein @ 2008-04-11  4:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, bergner

Hello, I am resubmitting this patch for review. To recap, when the
Backtrace function is called from a signal handler as a result of
invalid function pointer call, the unwinding code will itself raise
segv. This was first reported in
http://gcc.gnu.org/ml/gcc/2007-06/msg00329.html
I last submitted this patch for review in
http://gcc.gnu.org/ml/gcc-patches/2008-01/msg01474.html
but this was during a regression patch cycle.  If this patch is
accepted, would Peter Bergner mind checking it in?

This fix is for x86_64 and uses the mincore function determine if a
memory range is "safe" before attempting to read it, so that the
MD_FALLBACK_FRAME_STATE_FOR function will not segfault. If the memory
range is invalid, it is determined to be a invalid function pointer call
and the cfa is adjusted accordingly.

I tried porting the patch to POWER, the only other architecture I have
access to, but was unable to reproduce the segfault during backtrace.
Somehow the signal frame is causing the invalid address to be skipped.

Here is a test case to demonstrate the problem and verify the fix:
(This example uses a non-hard-coded address for the invalid function
pointer using mprotect'ed memory.)

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <execinfo.h>
#include <sys/mman.h>
#include <malloc.h>

void (*fp)(void);

int rc ;
struct sigaction newact;     /* Action */
struct sigaction oldact;     /* Old action */


void foo( void )
{
  char buf[16];

  buf[0] = '\n';
  buf[1] = '\0';
  printf( "Calling invalid function pointer.%s", buf );

  fp();
}

void catch_segv(int signum)
{
   int i, cnt;
   void *syms[100];
   char buf[80];
   char* *strs;

   rc = sigaction (SIGSEGV, &oldact, NULL);
   rc = sigaction (SIGBUS, &oldact, NULL);

   cnt = backtrace( syms, 100 );
   strs = backtrace_symbols( syms, 100 );

   for ( i = 0 ; i < cnt ; i++ )
   {
      snprintf( buf, sizeof(buf), "%d\t%lx\t%s\n",
       i,
       (unsigned long)syms[i],
       strs[i] );
      puts( buf );
   }
   free(strs);

   printf("PASS\n");
   exit(1);
}

int main( void )
{
   int result;
   
   fp = (void*) memalign(sysconf(_SC_PAGESIZE),4096);
   result = mprotect(fp, 4096, PROT_NONE);
   if (result) perror("mprotect");

   printf( "fp = %llx  (mprotect returned %d)\n", fp, result );

   newact.sa_handler = catch_segv;
   sigemptyset (&newact.sa_mask);
   newact.sa_flags = 0;

   rc = sigaction (SIGSEGV, &newact, &oldact);
   rc = sigaction (SIGBUS, &newact, &oldact);

   printf( "rc = %d, calling foo\n", rc );

   foo();

  return 0;
}


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


2008-04-10  Pete Eberlein  <eberlein@us.ibm.com>

	* 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 134151)
+++ gcc/config/i386/linux-unwind.h      (working copy)
@@ -39,6 +39,10 @@
 
 #define MD_FALLBACK_FRAME_STATE_FOR x86_64_fallback_frame_state
 
+#include <unistd.h>
+#include <sys/mman.h>
+
+
 static _Unwind_Reason_Code
 x86_64_fallback_frame_state (struct _Unwind_Context *context,
                             _Unwind_FrameState *fs)
@@ -46,7 +50,52 @@
   unsigned char *pc = context->ra;
   struct sigcontext *sc;
   long new_cfa;
+  int mem_invalid, saved_errno;
+  char dummy[2];
+  unsigned long start;
 
+  
+  /* check if memory is readable (using mincore) */
+  saved_errno = errno;
+  start = ((unsigned long)pc) & ~(sysconf(_SC_PAGE_SIZE) - 1);
+  mem_invalid = mincore((void*)start, (unsigned long)pc + 9 - start, dummy);
+  if (errno != ENOMEM && errno != EINVAL)
+    mem_invalid = 0;
+  errno = saved_errno;
+
+  if (mem_invalid)
+    {
+      /* 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)


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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 17:21           ` Jakub Jelinek
@ 2008-01-31 20:42             ` Daniel Jacobowitz
  0 siblings, 0 replies; 13+ messages in thread
From: Daniel Jacobowitz @ 2008-01-31 20:42 UTC (permalink / raw)
  To: gcc-patches

On Thu, Jan 31, 2008 at 08:36:02AM -0500, Jakub Jelinek wrote:
> On Thu, Jan 31, 2008 at 02:57:02PM +0100, Andi Kleen wrote:
> > > This seems to me like the cure is worse than the disease.
> > 
> > Then you should document somewhere that the unwinder is racy in this
> > regard.
> 
> Only for broken programs which munmap code pages which are still in use by
> some other thread.  Big deal.

It's racy if you unmap the unwinder, too :-)

-- 
Daniel Jacobowitz
CodeSourcery

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 16:19         ` Andi Kleen
@ 2008-01-31 17:21           ` Jakub Jelinek
  2008-01-31 20:42             ` Daniel Jacobowitz
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2008-01-31 17:21 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Haley, Pete Eberlein, gcc-patches

On Thu, Jan 31, 2008 at 02:57:02PM +0100, Andi Kleen wrote:
> > This seems to me like the cure is worse than the disease.
> 
> Then you should document somewhere that the unwinder is racy in this
> regard.

Only for broken programs which munmap code pages which are still in use by
some other thread.  Big deal.

	Jakub

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 14:24       ` Andrew Haley
@ 2008-01-31 16:19         ` Andi Kleen
  2008-01-31 17:21           ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Kleen @ 2008-01-31 16:19 UTC (permalink / raw)
  To: Andrew Haley; +Cc: Andi Kleen, Jakub Jelinek, Pete Eberlein, gcc-patches

> You could, but you shouldn't.  I agree with Jakub: we don't want to be
> messing with signal handlers.  For example, an application is perfectly
> entitled to enable/disable SIGSEGV handlers whenever it wants.

That would likely still work even with trampoline, but ok.
> 
> This seems to me like the cure is worse than the disease.

Then you should document somewhere that the unwinder is racy in this
regard.

-Andi

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 13:36     ` Andi Kleen
@ 2008-01-31 14:24       ` Andrew Haley
  2008-01-31 16:19         ` Andi Kleen
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Haley @ 2008-01-31 14:24 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Jakub Jelinek, Pete Eberlein, gcc-patches

Andi Kleen wrote:
> On Thu, Jan 31, 2008 at 05:53:59AM -0500, Jakub Jelinek wrote:
>> On Thu, Jan 31, 2008 at 11:48:18AM +0100, Andi Kleen wrote:
>>> mincore is racy because the page could be just munmaped by a different
>>> thread in the window between the call and the access. The only way to make it 
>>> safe would be to use mlock, but that might require root.
>>>
>>> Better would be to install a signal handler and handle the exception.
>>> Might be tricky to coordinate this with other signal handlers though.
>> That's not something the unwinder can/should do.  Signal handlers are
>> process-wide, a library shouldn't mess up with application's signal handlers
>> and some other thread can change the signal handler in the mean time anyway.
> 
> If you have glibc support you could certainly hook into sigaction() and
> friends and e.g. make all signal handlers go through a trampoline that
> checks for the unwind case.
> 
> In fact you could do it even without glibc support by overriding sigaction.

You could, but you shouldn't.  I agree with Jakub: we don't want to be
messing with signal handlers.  For example, an application is perfectly
entitled to enable/disable SIGSEGV handlers whenever it wants.

This seems to me like the cure is worse than the disease.

Andrew.

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 13:23   ` Jakub Jelinek
@ 2008-01-31 13:36     ` Andi Kleen
  2008-01-31 14:24       ` Andrew Haley
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Kleen @ 2008-01-31 13:36 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Andi Kleen, Pete Eberlein, gcc-patches

On Thu, Jan 31, 2008 at 05:53:59AM -0500, Jakub Jelinek wrote:
> On Thu, Jan 31, 2008 at 11:48:18AM +0100, Andi Kleen wrote:
> > mincore is racy because the page could be just munmaped by a different
> > thread in the window between the call and the access. The only way to make it 
> > safe would be to use mlock, but that might require root.
> > 
> > Better would be to install a signal handler and handle the exception.
> > Might be tricky to coordinate this with other signal handlers though.
> 
> That's not something the unwinder can/should do.  Signal handlers are
> process-wide, a library shouldn't mess up with application's signal handlers
> and some other thread can change the signal handler in the mean time anyway.

If you have glibc support you could certainly hook into sigaction() and
friends and e.g. make all signal handlers go through a trampoline that
checks for the unwind case.

In fact you could do it even without glibc support by overriding sigaction.

-Andi

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31 13:06 ` Andi Kleen
@ 2008-01-31 13:23   ` Jakub Jelinek
  2008-01-31 13:36     ` Andi Kleen
  0 siblings, 1 reply; 13+ messages in thread
From: Jakub Jelinek @ 2008-01-31 13:23 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Pete Eberlein, gcc-patches

On Thu, Jan 31, 2008 at 11:48:18AM +0100, Andi Kleen wrote:
> mincore is racy because the page could be just munmaped by a different
> thread in the window between the call and the access. The only way to make it 
> safe would be to use mlock, but that might require root.
> 
> Better would be to install a signal handler and handle the exception.
> Might be tricky to coordinate this with other signal handlers though.

That's not something the unwinder can/should do.  Signal handlers are
process-wide, a library shouldn't mess up with application's signal handlers
and some other thread can change the signal handler in the mean time anyway.

	Jakub

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
       [not found] <47A0F9FA.9000903@linux.vnet.ibm.com.suse.lists.egcs-patches>
@ 2008-01-31 13:06 ` Andi Kleen
  2008-01-31 13:23   ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Andi Kleen @ 2008-01-31 13:06 UTC (permalink / raw)
  To: Pete Eberlein; +Cc: gcc-patches

Pete Eberlein <eberlein@linux.vnet.ibm.com> writes:

> Hello, I am resubmitting this patch for review.  To recap, when the
> Backtrace function is called from a signal handler as a result of
> invalid function pointer call, the unwinding code will itself raise
> segv.  This was first reported in
> http://gcc.gnu.org/ml/gcc/2007-06/msg00329.html
>
> This fix is for x86_64 and uses the mincore function determine if a
> memory range is "safe" before attempting to read it, so that the
> MD_FALLBACK_FRAME_STATE_FOR function will not segfault.  If the memory
> range is invalid, it is determined to be a invalid function pointer
> call and the cfa is adjusted accordingly.

mincore is racy because the page could be just munmaped by a different
thread in the window between the call and the access. The only way to make it 
safe would be to use mlock, but that might require root.

Better would be to install a signal handler and handle the exception.
Might be tricky to coordinate this with other signal handlers though.

-Andi

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

* Re: [PATCH] Segfault while unwinding an invalid function pointer
  2008-01-31  8:48 Pete Eberlein
@ 2008-01-31  9:27 ` Jakub Jelinek
  0 siblings, 0 replies; 13+ messages in thread
From: Jakub Jelinek @ 2008-01-31  9:27 UTC (permalink / raw)
  To: eberlein; +Cc: gcc-patches, drow, bergner

On Wed, Jan 30, 2008 at 02:28:10PM -0800, Pete Eberlein wrote:
> 2008-01-30  Pete Eberlein  <eberlein@us.ibm.com>
> 
>     * linux-unwind.h (x86_64_fallback_frame_state): Handle
>     invalid function pointer address and change cfa.

This isn't a regression, at this point this should wait for 4.4 stage1
and perhaps can be backported to 4.3.1.
Would be good to patch a bunch of other major linux targets though,
there is nothing x86_64 specific on it.

	Jakub

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

* [PATCH] Segfault while unwinding an invalid function pointer
@ 2008-01-31  8:48 Pete Eberlein
  2008-01-31  9:27 ` Jakub Jelinek
  0 siblings, 1 reply; 13+ messages in thread
From: Pete Eberlein @ 2008-01-31  8:48 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, drow, bergner

Hello, I am resubmitting this patch for review.  To recap, when the Backtrace 
function is called from a signal handler as a result of invalid function pointer 
call, the unwinding code will itself raise segv.  This was first reported in 
http://gcc.gnu.org/ml/gcc/2007-06/msg00329.html

This fix is for x86_64 and uses the mincore function determine if a memory range 
is "safe" before attempting to read it, so that the MD_FALLBACK_FRAME_STATE_FOR 
function will not segfault.  If the memory range is invalid, it is determined to 
be a invalid function pointer call and the cfa is adjusted accordingly.

Here is a test case to demonstrate the problem and verify the fix:

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <execinfo.h>

void (*fp)(void) = (void *)0xffffff;

int rc ;
struct sigaction newact;     /* Action */
struct sigaction oldact;     /* Old action */


void foo( void )
{
   char buf[16];

   buf[0] = '\n';
   buf[1] = '\0';
   printf( "Calling invalid function pointer.%s", buf );

   fp();
}

void catch_segv(int signum)
{
    int i, cnt;
    void *syms[100];
    char buf[80];
    char* *strs;

    rc = sigaction (SIGSEGV, &oldact, NULL);
    rc = sigaction (SIGBUS, &oldact, NULL);

    cnt = backtrace( syms, 100 );
    strs = backtrace_symbols( syms, 100 );

    for ( i = 0 ; i < cnt ; i++ )
    {
       snprintf( buf, sizeof(buf), "%d\t%lx\t%s\n",
        i,
        (unsigned long)syms[i],
        strs[i] );
       puts( buf );
    }
    free(strs);

    printf("PASS\n");
    exit(1);
}

int main( void )
{
    newact.sa_handler = catch_segv;
    sigemptyset (&newact.sa_mask);
    newact.sa_flags = 0;

    rc = sigaction (SIGSEGV, &newact, &oldact);
    rc = sigaction (SIGBUS, &newact, &oldact);

    printf( "rc = %d, calling foo\n", rc );

    foo();

   return 0;
}


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


2008-01-30  Pete Eberlein  <eberlein@us.ibm.com>

     * 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 131968)
+++ gcc/config/i386/linux-unwind.h      (working copy)
@@ -37,6 +37,9 @@
  #include <signal.h>
  #include <sys/ucontext.h>

+#include <unistd.h>
+#include <sys/mman.h>
+
  #define MD_FALLBACK_FRAME_STATE_FOR x86_64_fallback_frame_state

  static _Unwind_Reason_Code
@@ -46,7 +49,51 @@
    unsigned char *pc = context->ra;
    struct sigcontext *sc;
    long new_cfa;
+  int mem_invalid, saved_errno;
+  char dummy[2];
+  unsigned long start;

+  /* check if memory is readable (using mincore) */
+  saved_errno = errno;
+  start = ((unsigned long)pc) & ~(sysconf(_SC_PAGE_SIZE) - 1);
+  mem_invalid = mincore((void*)start, (unsigned long)(pc+9-start), dummy);
+  if (errno != ENOMEM && errno != EINVAL)
+    mem_invalid = 0;
+  errno = saved_errno;
+
+  if (mem_invalid)
+    {
+      /* 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)



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

* [PATCH] Segfault while unwinding an invalid function pointer
@ 2007-10-10 20:39 Pete Eberlein
  0 siblings, 0 replies; 13+ messages in thread
From: Pete Eberlein @ 2007-10-10 20:39 UTC (permalink / raw)
  To: gcc-patches

When the Backtrace function is called from a signal handler as a result 
of invalid function pointer call, the unwinding code will itself raise 
segv.  This was first reported in 
http://gcc.gnu.org/ml/gcc/2007-06/msg00329.html

This fix is for x86_64 and adds a function that will determine if a 
memory range is "safe" before attempting to read it, so that the 
MD_FALLBACK_FRAME_STATE_FOR function will not segfault.  If the memory 
range is invalid, it is determined to be a invalid function pointer call 
and the cfa is adjusted accordingly.  The function which determines if a 
memory range is "safe" does not install a signal handler; instead it 
writes the memory range to a file and checks the result of bytes written 
to see if the memory was valid.  Since the writes happen in kernel 
space, no segfault will be generated.

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


2007-10-10  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 129215)
+++ gcc/config/i386/linux-unwind.h      (working copy)
@@ -39,6 +39,65 @@

  #define MD_FALLBACK_FRAME_STATE_FOR x86_64_fallback_frame_state

+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#define tmp_path "/tmp/null"
+
+static int
+unwind_memory_range_check (void *addr, size_t len)
+{
+  int rc;
+  int result = 0;
+  int tmp_null_f = -1;
+  int write_len = 0;
+  mode_t mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+
+  errno = 0;
+  if (tmp_null_f == -1)
+  {
+    tmp_null_f = open (tmp_path, (O_WRONLY | O_CREAT | O_TRUNC), mode);
+    if (tmp_null_f == -1) {
+      perror (" open (" tmp_path ") failed\n");
+      exit (1);
+    }
+  }
+  if (tmp_null_f != -1)
+  {
+    if ((write_len + len) > 4096)
+    {
+      if (write_len > 4096)
+      {
+        errno = 0;
+        rc = ftruncate (tmp_null_f, 0);
+        if (rc == -1)
+        {
+          perror ("ftruncate failed\n");
+        }
+      }
+      write_len = 0;
+      errno = 0;
+      rc = lseek (tmp_null_f, 0, SEEK_SET);
+
+      if (rc == -1)
+      {
+        perror ("lseek failed\n");
+      }
+    }
+    write_len += len;
+
+    rc = write (tmp_null_f, addr, len);
+    result = (rc == len);
+
+    close (tmp_null_f);
+    unlink (tmp_path);
+
+  }
+  return result;
+}
+
  static _Unwind_Reason_Code
  x86_64_fallback_frame_state (struct _Unwind_Context *context,
                              _Unwind_FrameState *fs)
@@ -47,6 +106,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)

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

end of thread, other threads:[~2008-04-11  0:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <OF41DBA6D6.22A62FDD-ON8825738B.00651D50-8825738B.006528AE@us.ibm.com>
     [not found] ` <4730D908.8090401@linux.vnet.ibm.com>
2007-11-06 21:18   ` [PATCH] Segfault while unwinding an invalid function pointer Pete Eberlein
2007-11-16 22:25     ` Pete Eberlein
2008-04-11  4:23 Pete Eberlein
     [not found] <47A0F9FA.9000903@linux.vnet.ibm.com.suse.lists.egcs-patches>
2008-01-31 13:06 ` Andi Kleen
2008-01-31 13:23   ` Jakub Jelinek
2008-01-31 13:36     ` Andi Kleen
2008-01-31 14:24       ` Andrew Haley
2008-01-31 16:19         ` Andi Kleen
2008-01-31 17:21           ` Jakub Jelinek
2008-01-31 20:42             ` Daniel Jacobowitz
  -- strict thread matches above, loose matches on Subject: below --
2008-01-31  8:48 Pete Eberlein
2008-01-31  9:27 ` Jakub Jelinek
2007-10-10 20:39 Pete Eberlein

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