Revision 2aeb1883547626d82c597cce2c99f0b9c62e2425 authored by Jiri Olsa on 20 July 2017, 14:14:55 UTC, committed by Ingo Molnar on 21 July 2017, 07:54:23 UTC
We're missing ctx lock when iterating children siblings within the perf_read path for group reading. Following race and crash can happen: User space doing read syscall on event group leader: T1: perf_read lock event->ctx->mutex perf_read_group lock leader->child_mutex __perf_read_group_add(child) list_for_each_entry(sub, &leader->sibling_list, group_entry) ----> sub might be invalid at this point, because it could get removed via perf_event_exit_task_context in T2 Child exiting and cleaning up its events: T2: perf_event_exit_task_context lock ctx->mutex list_for_each_entry_safe(child_event, next, &child_ctx->event_list,... perf_event_exit_event(child) lock ctx->lock perf_group_detach(child) unlock ctx->lock ----> child is removed from sibling_list without any sync with T1 path above ... free_event(child) Before the child is removed from the leader's child_list, (and thus is omitted from perf_read_group processing), we need to ensure that perf_read_group touches child's siblings under its ctx->lock. Peter further notes: | One additional note; this bug got exposed by commit: | | ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP") | | which made it possible to actually trigger this code-path. Tested-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Fixes: ba5213ae6b88 ("perf/core: Correct event creation with PERF_FORMAT_GROUP") Link: http://lkml.kernel.org/r/20170720141455.2106-1-jolsa@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent 3bda69c
amdtp-stream-trace.h
/*
* amdtp-stream-trace.h - tracepoint definitions to dump a part of packet data
*
* Copyright (c) 2016 Takashi Sakamoto
* Licensed under the terms of the GNU General Public License, version 2.
*/
#undef TRACE_SYSTEM
#define TRACE_SYSTEM snd_firewire_lib
#if !defined(_AMDTP_STREAM_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
#define _AMDTP_STREAM_TRACE_H
#include <linux/tracepoint.h>
TRACE_EVENT(in_packet,
TP_PROTO(const struct amdtp_stream *s, u32 cycles, u32 cip_header[2], unsigned int payload_length, unsigned int index),
TP_ARGS(s, cycles, cip_header, payload_length, index),
TP_STRUCT__entry(
__field(unsigned int, second)
__field(unsigned int, cycle)
__field(int, channel)
__field(int, src)
__field(int, dest)
__field(u32, cip_header0)
__field(u32, cip_header1)
__field(unsigned int, payload_quadlets)
__field(unsigned int, packet_index)
__field(unsigned int, irq)
__field(unsigned int, index)
),
TP_fast_assign(
__entry->second = cycles / CYCLES_PER_SECOND;
__entry->cycle = cycles % CYCLES_PER_SECOND;
__entry->channel = s->context->channel;
__entry->src = fw_parent_device(s->unit)->node_id;
__entry->dest = fw_parent_device(s->unit)->card->node_id;
__entry->cip_header0 = cip_header[0];
__entry->cip_header1 = cip_header[1];
__entry->payload_quadlets = payload_length / 4;
__entry->packet_index = s->packet_index;
__entry->irq = !!in_interrupt();
__entry->index = index;
),
TP_printk(
"%02u %04u %04x %04x %02d %08x %08x %03u %02u %01u %02u",
__entry->second,
__entry->cycle,
__entry->src,
__entry->dest,
__entry->channel,
__entry->cip_header0,
__entry->cip_header1,
__entry->payload_quadlets,
__entry->packet_index,
__entry->irq,
__entry->index)
);
TRACE_EVENT(out_packet,
TP_PROTO(const struct amdtp_stream *s, u32 cycles, __be32 *cip_header, unsigned int payload_length, unsigned int index),
TP_ARGS(s, cycles, cip_header, payload_length, index),
TP_STRUCT__entry(
__field(unsigned int, second)
__field(unsigned int, cycle)
__field(int, channel)
__field(int, src)
__field(int, dest)
__field(u32, cip_header0)
__field(u32, cip_header1)
__field(unsigned int, payload_quadlets)
__field(unsigned int, packet_index)
__field(unsigned int, irq)
__field(unsigned int, index)
),
TP_fast_assign(
__entry->second = cycles / CYCLES_PER_SECOND;
__entry->cycle = cycles % CYCLES_PER_SECOND;
__entry->channel = s->context->channel;
__entry->src = fw_parent_device(s->unit)->card->node_id;
__entry->dest = fw_parent_device(s->unit)->node_id;
__entry->cip_header0 = be32_to_cpu(cip_header[0]);
__entry->cip_header1 = be32_to_cpu(cip_header[1]);
__entry->payload_quadlets = payload_length / 4;
__entry->packet_index = s->packet_index;
__entry->irq = !!in_interrupt();
__entry->index = index;
),
TP_printk(
"%02u %04u %04x %04x %02d %08x %08x %03u %02u %01u %02u",
__entry->second,
__entry->cycle,
__entry->src,
__entry->dest,
__entry->channel,
__entry->cip_header0,
__entry->cip_header1,
__entry->payload_quadlets,
__entry->packet_index,
__entry->irq,
__entry->index)
);
TRACE_EVENT(in_packet_without_header,
TP_PROTO(const struct amdtp_stream *s, u32 cycles, unsigned int payload_quadlets, unsigned int data_blocks, unsigned int index),
TP_ARGS(s, cycles, payload_quadlets, data_blocks, index),
TP_STRUCT__entry(
__field(unsigned int, second)
__field(unsigned int, cycle)
__field(int, channel)
__field(int, src)
__field(int, dest)
__field(unsigned int, payload_quadlets)
__field(unsigned int, data_blocks)
__field(unsigned int, data_block_counter)
__field(unsigned int, packet_index)
__field(unsigned int, irq)
__field(unsigned int, index)
),
TP_fast_assign(
__entry->second = cycles / CYCLES_PER_SECOND;
__entry->cycle = cycles % CYCLES_PER_SECOND;
__entry->channel = s->context->channel;
__entry->src = fw_parent_device(s->unit)->node_id;
__entry->dest = fw_parent_device(s->unit)->card->node_id;
__entry->payload_quadlets = payload_quadlets;
__entry->data_blocks = data_blocks,
__entry->data_block_counter = s->data_block_counter,
__entry->packet_index = s->packet_index;
__entry->irq = !!in_interrupt();
__entry->index = index;
),
TP_printk(
"%02u %04u %04x %04x %02d %03u %3u %3u %02u %01u %02u",
__entry->second,
__entry->cycle,
__entry->src,
__entry->dest,
__entry->channel,
__entry->payload_quadlets,
__entry->data_blocks,
__entry->data_block_counter,
__entry->packet_index,
__entry->irq,
__entry->index)
);
TRACE_EVENT(out_packet_without_header,
TP_PROTO(const struct amdtp_stream *s, u32 cycles, unsigned int payload_length, unsigned int data_blocks, unsigned int index),
TP_ARGS(s, cycles, payload_length, data_blocks, index),
TP_STRUCT__entry(
__field(unsigned int, second)
__field(unsigned int, cycle)
__field(int, channel)
__field(int, src)
__field(int, dest)
__field(unsigned int, payload_quadlets)
__field(unsigned int, data_blocks)
__field(unsigned int, data_block_counter)
__field(unsigned int, packet_index)
__field(unsigned int, irq)
__field(unsigned int, index)
),
TP_fast_assign(
__entry->second = cycles / CYCLES_PER_SECOND;
__entry->cycle = cycles % CYCLES_PER_SECOND;
__entry->channel = s->context->channel;
__entry->src = fw_parent_device(s->unit)->card->node_id;
__entry->dest = fw_parent_device(s->unit)->node_id;
__entry->payload_quadlets = payload_length / 4;
__entry->data_blocks = data_blocks,
__entry->data_blocks = s->data_block_counter,
__entry->packet_index = s->packet_index;
__entry->irq = !!in_interrupt();
__entry->index = index;
),
TP_printk(
"%02u %04u %04x %04x %02d %03u %02u %03u %02u %01u %02u",
__entry->second,
__entry->cycle,
__entry->src,
__entry->dest,
__entry->channel,
__entry->payload_quadlets,
__entry->data_blocks,
__entry->data_block_counter,
__entry->packet_index,
__entry->irq,
__entry->index)
);
#endif
#undef TRACE_INCLUDE_PATH
#define TRACE_INCLUDE_PATH .
#undef TRACE_INCLUDE_FILE
#define TRACE_INCLUDE_FILE amdtp-stream-trace
#include <trace/define_trace.h>
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...