On Thu, 2015-06-11 at 00:30 -0400, Vince Weaver wrote:
> On Fri, 29 May 2015, Ingo Molnar wrote:
>
> > * Vince Weaver <vincent.wea...@maine.edu> wrote:
>
> > > If we inherit events, we inherit the signal state but not the fasync
> > > state, so
> > > overflows in inherited children will never trigger the signal handler.
> > >
> > > Signed-off-by: Vince Weaver <vincent.wea...@maine.edu>
> > >
> > > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > > index 1a3bf48..7df4cf5 100644
> > > --- a/kernel/events/core.c
> > > +++ b/kernel/events/core.c
> > > @@ -8626,6 +8630,8 @@ inherit_event(struct perf_event *parent_event,
> > > child_event->overflow_handler_context
> > > = parent_event->overflow_handler_context;
> > >
> > > + child_event->fasync = parent_event->fasync;
> > > +
> > > /*
> > > * Precalculate sample_data sizes
> > > */
>
> This patch, while it does work well enough to enable self-monitored-sampling
> of OpenMP programs, falls apart under fuzzing.
>
> You end up with lots of
>
> [25592.289382] kill_fasync: bad magic number in fasync_struct!
>
> warnings and eventually I managed to lock up the system that way.
Right, I had a peek earlier at how fasync worked but came away confused.
Today I seem to have had better luck. Installing fasync allocates memory
and sets filp->f_flags |= FASYNC, which upon the demise of the file
descriptor ensures the allocation is freed.
Now for perf, we can have the events stick around for a while after the
original FD is dead because of references from child events. With the
above patch these events would still have a pointer into this free'd
fasync. This is bad.
A further problem with the patch is that if the parent changes its
fasync state the children might lag and again have pointers into dead
space.
All is not lost though; does something like the below work?
---
kernel/events/core.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1e33b9141f03..057f599ae0dc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4742,12 +4742,20 @@ static const struct file_operations perf_fops = {
* to user-space before waking everybody up.
*/
+static inline struct fasync_struct **perf_event_fasync(struct perf_event
*event)
+{
+ /* only the parent has fasync state */
+ if (event->parent)
+ event = event->parent;
+ return &event->fasync;
+}
+
void perf_event_wakeup(struct perf_event *event)
{
ring_buffer_wakeup(event);
if (event->pending_kill) {
- kill_fasync(&event->fasync, SIGIO, event->pending_kill);
+ kill_fasync(perf_event_fasync(event), SIGIO,
event->pending_kill);
event->pending_kill = 0;
}
}
@@ -6126,7 +6134,7 @@ static int __perf_event_overflow(struct perf_event *event,
else
perf_event_output(event, data, regs);
- if (event->fasync && event->pending_kill) {
+ if (*perf_event_fasync(event) && event->pending_kill) {
event->pending_wakeup = 1;
irq_work_queue(&event->pending);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
