All of lore.kernel.org
 help / color / mirror / Atom feed
* perf: overflow signal survives an exec call starting in 3.0
@ 2011-08-23 15:36 Vince Weaver
  2011-08-25 14:37 ` Peter Zijlstra
  2011-08-25 14:47 ` Peter Zijlstra
  0 siblings, 2 replies; 3+ messages in thread
From: Vince Weaver @ 2011-08-23 15:36 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, mingo, paulus, acme, Stephane Eranian, krentel

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

Hello

Mark Krentel noticed that starting with Linux 3.0 perf_event signals
survive a call to exec().  

This means that if you exec() from within a perf-monitored process
and don't immediately start a signal handler, your process will
quickly be killed with a SIGIO signal.

I'm guessing this was an unintended change, although what to do in
this situation is a bit vague.

I tediously bisected this to the following commit:

commit f506b3dc0ec454a16d40cab9ee5d75435b39dc50
Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date:   Thu May 26 17:02:53 2011 +0200

    perf: Fix SIGIO handling

Attached is an example program that exhibits the problem.

Vince

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: signal_after_exec.c --]
[-- Type: text/x-csrc; name="signal_after_exec.c", Size: 3036 bytes --]

/* signal_after_exec.c  */
/* by Vince Weaver   vweaver1 _at_ eecs.utk.edu */

/* Compile with gcc -O2 -Wall -o signal_after_exec signal_after_exec.c */

/* On 2.6.39 and earlier the execd process gets no signals               */
/* On 3.0 and normal it does, which quickly kills the program with SIGIO */

#define _GNU_SOURCE 1

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include <unistd.h>
#include <fcntl.h>

#include <signal.h>

#include <sys/mman.h>

#include <sys/ioctl.h>

#include <asm/unistd.h>
#include <sys/prctl.h>
#include "perf_event.h"

int perf_event_open(struct perf_event_attr *hw_event_uptr,
                    pid_t pid, int cpu, int group_fd, unsigned long flags) {
   
  return syscall(__NR_perf_event_open,hw_event_uptr, pid, cpu,
		 group_fd, flags);
}


static int count=0;

static void our_handler(int signum,siginfo_t *oh, void *blah) {
   
   int ret,fd1;
   
   fd1=oh->si_fd;
   
   ret=ioctl(fd1, PERF_EVENT_IOC_DISABLE,0);
   
   count++;
   
   ret=ioctl(fd1, PERF_EVENT_IOC_REFRESH,1);
}

double busywork(int count) {
 
  int i;
  double sum=0.0012;
   
  for(i=0;i<count;i++) {
    sum+=0.01;
  }
  return sum;
   
}


int main(int argc, char** argv) {
   
  int fd,ret;
  double result;

   struct perf_event_attr pe;
   struct sigaction sa;

   if (argc>1) {
      result=busywork(10000000);
      printf("Count after exec=%d (%lf)\n",count,result);
      exit(0);
   }

   printf("\nOn 2.6.39 and earlier the exec'd process gets no signals.\n");
   printf("On 3.0 and later the exec'd process gets a signal, which\n");
   printf("  is not handled and it dies with SIGIO.\n\n");

   /* set up signal handler */
   memset(&sa, 0, sizeof(struct sigaction));
   sa.sa_sigaction = our_handler;
   sa.sa_flags = SA_SIGINFO;

   if (sigaction( SIGIO, &sa, NULL) < 0) {
     fprintf(stderr,"Error setting up signal handler\n");
     exit(1);
   }

   memset(&pe,0,sizeof(struct perf_event_attr));

   pe.type=PERF_TYPE_HARDWARE;
   pe.size=sizeof(struct perf_event_attr);
   pe.config=PERF_COUNT_HW_INSTRUCTIONS;
   pe.sample_period=100000;
   pe.sample_type=PERF_SAMPLE_IP;
   pe.read_format=0;
   pe.disabled=1;
   pe.pinned=1;
 
   pe.wakeup_events=1;
   
   fd=perf_event_open(&pe,0,-1,-1,0);
   if (fd<0) {
      fprintf(stderr,"Error opening\n");
      exit(1);
   }

   void *blargh;
   
   blargh=mmap(NULL, (1+1)*4096, 
	       PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);


   /* setup event 2 to have overflow signals */
   fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
   fcntl(fd, F_SETSIG, SIGIO);
   fcntl(fd, F_SETOWN,getpid());
   
   ioctl(fd, PERF_EVENT_IOC_RESET, 0);   

   /* enable counting */
   ret=ioctl(fd, PERF_EVENT_IOC_ENABLE,0);

   result=busywork(10000000);

   printf("Count before exec=%d (%lf)\n",count,result);

   /* exec ourselves, but call a busy function */
   execl(argv[0],argv[0],"busy",NULL);

   return 0;
}

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

* Re: perf: overflow signal survives an exec call starting in 3.0
  2011-08-23 15:36 perf: overflow signal survives an exec call starting in 3.0 Vince Weaver
@ 2011-08-25 14:37 ` Peter Zijlstra
  2011-08-25 14:47 ` Peter Zijlstra
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Zijlstra @ 2011-08-25 14:37 UTC (permalink / raw)
  To: Vince Weaver; +Cc: linux-kernel, mingo, paulus, acme, Stephane Eranian, krentel

On Tue, 2011-08-23 at 11:36 -0400, Vince Weaver wrote:
> I'm guessing this was an unintended change, although what to do in
> this situation is a bit vague.

> 
> commit f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Thu May 26 17:02:53 2011 +0200
> 
>     perf: Fix SIGIO handling 

Ah, quite... its a combination of way too many weird things.

So that change made it possible (as requested) to receive a signal
without having to mmap() the buffer, which seems like a perfectly sane
thing.

Now, looking at fs/exec.c:setup_new_exec() we see that we only call
perf_event_exit_task() when we cross a security domain. 

Previously the exec() would have wiped out the mmap, and hence the
counter would have stopped sending signals (but not stopped counting).

Now exec() doesn't normally close fds, so the counter stays around and
with the new scheme continues sending signals.

I think the message is, don't do that then.. and use fcntl(fd, F_SETFD,
FD_CLOEXEC) or so.


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

* Re: perf: overflow signal survives an exec call starting in 3.0
  2011-08-23 15:36 perf: overflow signal survives an exec call starting in 3.0 Vince Weaver
  2011-08-25 14:37 ` Peter Zijlstra
@ 2011-08-25 14:47 ` Peter Zijlstra
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Zijlstra @ 2011-08-25 14:47 UTC (permalink / raw)
  To: Vince Weaver; +Cc: linux-kernel, mingo, paulus, acme, Stephane Eranian, krentel

On Thu, 2011-08-25 at 16:37 +0200, Peter Zijlstra wrote:
> On Tue, 2011-08-23 at 11:36 -0400, Vince Weaver wrote:
> > I'm guessing this was an unintended change, although what to do in
> > this situation is a bit vague.
> 
> > 
> > commit f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> > Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Date:   Thu May 26 17:02:53 2011 +0200
> > 
> >     perf: Fix SIGIO handling 
> 
> Ah, quite... its a combination of way too many weird things.
> 
> So that change made it possible (as requested) to receive a signal
> without having to mmap() the buffer, which seems like a perfectly sane
> thing.
> 
> Now, looking at fs/exec.c:setup_new_exec() we see that we only call
> perf_event_exit_task() when we cross a security domain. 
> 
> Previously the exec() would have wiped out the mmap, and hence the
> counter would have stopped sending signals (but not stopped counting).
> 
> Now exec() doesn't normally close fds, so the counter stays around and
> with the new scheme continues sending signals.
> 
> I think the message is, don't do that then.. and use fcntl(fd, F_SETFD,
> FD_CLOEXEC) or so.

That is, I suspect you can get into the same trouble by creating a pipe,
setting F_SETOWN on the read side and then calling exec() while ensuring
someone writes to the thing at the right moment.



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

end of thread, other threads:[~2011-08-25 14:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-23 15:36 perf: overflow signal survives an exec call starting in 3.0 Vince Weaver
2011-08-25 14:37 ` Peter Zijlstra
2011-08-25 14:47 ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.