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
fireworks_stream.c
/*
* fireworks_stream.c - a part of driver for Fireworks based devices
*
* Copyright (c) 2013-2014 Takashi Sakamoto
*
* Licensed under the terms of the GNU General Public License, version 2.
*/
#include "./fireworks.h"
#define CALLBACK_TIMEOUT 100
static int
init_stream(struct snd_efw *efw, struct amdtp_stream *stream)
{
struct cmp_connection *conn;
enum cmp_direction c_dir;
enum amdtp_stream_direction s_dir;
int err;
if (stream == &efw->tx_stream) {
conn = &efw->out_conn;
c_dir = CMP_OUTPUT;
s_dir = AMDTP_IN_STREAM;
} else {
conn = &efw->in_conn;
c_dir = CMP_INPUT;
s_dir = AMDTP_OUT_STREAM;
}
err = cmp_connection_init(conn, efw->unit, c_dir, 0);
if (err < 0)
goto end;
err = amdtp_am824_init(stream, efw->unit, s_dir, CIP_BLOCKING);
if (err < 0) {
amdtp_stream_destroy(stream);
cmp_connection_destroy(conn);
}
end:
return err;
}
static void
stop_stream(struct snd_efw *efw, struct amdtp_stream *stream)
{
amdtp_stream_pcm_abort(stream);
amdtp_stream_stop(stream);
if (stream == &efw->tx_stream)
cmp_connection_break(&efw->out_conn);
else
cmp_connection_break(&efw->in_conn);
}
static int
start_stream(struct snd_efw *efw, struct amdtp_stream *stream,
unsigned int sampling_rate)
{
struct cmp_connection *conn;
unsigned int mode, pcm_channels, midi_ports;
int err;
err = snd_efw_get_multiplier_mode(sampling_rate, &mode);
if (err < 0)
goto end;
if (stream == &efw->tx_stream) {
conn = &efw->out_conn;
pcm_channels = efw->pcm_capture_channels[mode];
midi_ports = efw->midi_out_ports;
} else {
conn = &efw->in_conn;
pcm_channels = efw->pcm_playback_channels[mode];
midi_ports = efw->midi_in_ports;
}
err = amdtp_am824_set_parameters(stream, sampling_rate,
pcm_channels, midi_ports, false);
if (err < 0)
goto end;
/* establish connection via CMP */
err = cmp_connection_establish(conn,
amdtp_stream_get_max_payload(stream));
if (err < 0)
goto end;
/* start amdtp stream */
err = amdtp_stream_start(stream,
conn->resources.channel,
conn->speed);
if (err < 0) {
stop_stream(efw, stream);
goto end;
}
/* wait first callback */
if (!amdtp_stream_wait_callback(stream, CALLBACK_TIMEOUT)) {
stop_stream(efw, stream);
err = -ETIMEDOUT;
}
end:
return err;
}
/*
* This function should be called before starting the stream or after stopping
* the streams.
*/
static void
destroy_stream(struct snd_efw *efw, struct amdtp_stream *stream)
{
struct cmp_connection *conn;
if (stream == &efw->tx_stream)
conn = &efw->out_conn;
else
conn = &efw->in_conn;
amdtp_stream_destroy(stream);
cmp_connection_destroy(conn);
}
static int
check_connection_used_by_others(struct snd_efw *efw, struct amdtp_stream *s)
{
struct cmp_connection *conn;
bool used;
int err;
if (s == &efw->tx_stream)
conn = &efw->out_conn;
else
conn = &efw->in_conn;
err = cmp_connection_check_used(conn, &used);
if ((err >= 0) && used && !amdtp_stream_running(s)) {
dev_err(&efw->unit->device,
"Connection established by others: %cPCR[%d]\n",
(conn->direction == CMP_OUTPUT) ? 'o' : 'i',
conn->pcr_index);
err = -EBUSY;
}
return err;
}
int snd_efw_stream_init_duplex(struct snd_efw *efw)
{
int err;
err = init_stream(efw, &efw->tx_stream);
if (err < 0)
goto end;
/* Fireworks transmits NODATA packets with TAG0. */
efw->tx_stream.flags |= CIP_EMPTY_WITH_TAG0;
/* Fireworks has its own meaning for dbc. */
efw->tx_stream.flags |= CIP_DBC_IS_END_EVENT;
/* Fireworks reset dbc at bus reset. */
efw->tx_stream.flags |= CIP_SKIP_DBC_ZERO_CHECK;
/*
* But Recent firmwares starts packets with non-zero dbc.
* Driver version 5.7.6 installs firmware version 5.7.3.
*/
if (efw->is_fireworks3 &&
(efw->firmware_version == 0x5070000 ||
efw->firmware_version == 0x5070300 ||
efw->firmware_version == 0x5080000))
efw->tx_stream.tx_first_dbc = 0x02;
/* AudioFire9 always reports wrong dbs. */
if (efw->is_af9)
efw->tx_stream.flags |= CIP_WRONG_DBS;
/* Firmware version 5.5 reports fixed interval for dbc. */
if (efw->firmware_version == 0x5050000)
efw->tx_stream.tx_dbc_interval = 8;
err = init_stream(efw, &efw->rx_stream);
if (err < 0) {
destroy_stream(efw, &efw->tx_stream);
goto end;
}
/* set IEC61883 compliant mode (actually not fully compliant...) */
err = snd_efw_command_set_tx_mode(efw, SND_EFW_TRANSPORT_MODE_IEC61883);
if (err < 0) {
destroy_stream(efw, &efw->tx_stream);
destroy_stream(efw, &efw->rx_stream);
}
end:
return err;
}
int snd_efw_stream_start_duplex(struct snd_efw *efw, unsigned int rate)
{
unsigned int curr_rate;
int err = 0;
/* Need no substreams */
if (efw->playback_substreams == 0 && efw->capture_substreams == 0)
goto end;
/*
* Considering JACK/FFADO streaming:
* TODO: This can be removed hwdep functionality becomes popular.
*/
err = check_connection_used_by_others(efw, &efw->rx_stream);
if (err < 0)
goto end;
/* packet queueing error */
if (amdtp_streaming_error(&efw->tx_stream))
stop_stream(efw, &efw->tx_stream);
if (amdtp_streaming_error(&efw->rx_stream))
stop_stream(efw, &efw->rx_stream);
/* stop streams if rate is different */
err = snd_efw_command_get_sampling_rate(efw, &curr_rate);
if (err < 0)
goto end;
if (rate == 0)
rate = curr_rate;
if (rate != curr_rate) {
stop_stream(efw, &efw->tx_stream);
stop_stream(efw, &efw->rx_stream);
}
/* master should be always running */
if (!amdtp_stream_running(&efw->rx_stream)) {
err = snd_efw_command_set_sampling_rate(efw, rate);
if (err < 0)
goto end;
err = start_stream(efw, &efw->rx_stream, rate);
if (err < 0) {
dev_err(&efw->unit->device,
"fail to start AMDTP master stream:%d\n", err);
goto end;
}
}
/* start slave if needed */
if (efw->capture_substreams > 0 &&
!amdtp_stream_running(&efw->tx_stream)) {
err = start_stream(efw, &efw->tx_stream, rate);
if (err < 0) {
dev_err(&efw->unit->device,
"fail to start AMDTP slave stream:%d\n", err);
stop_stream(efw, &efw->rx_stream);
}
}
end:
return err;
}
void snd_efw_stream_stop_duplex(struct snd_efw *efw)
{
if (efw->capture_substreams == 0) {
stop_stream(efw, &efw->tx_stream);
if (efw->playback_substreams == 0)
stop_stream(efw, &efw->rx_stream);
}
}
void snd_efw_stream_update_duplex(struct snd_efw *efw)
{
if (cmp_connection_update(&efw->out_conn) < 0 ||
cmp_connection_update(&efw->in_conn) < 0) {
stop_stream(efw, &efw->rx_stream);
stop_stream(efw, &efw->tx_stream);
} else {
amdtp_stream_update(&efw->rx_stream);
amdtp_stream_update(&efw->tx_stream);
}
}
void snd_efw_stream_destroy_duplex(struct snd_efw *efw)
{
destroy_stream(efw, &efw->rx_stream);
destroy_stream(efw, &efw->tx_stream);
}
void snd_efw_stream_lock_changed(struct snd_efw *efw)
{
efw->dev_lock_changed = true;
wake_up(&efw->hwdep_wait);
}
int snd_efw_stream_lock_try(struct snd_efw *efw)
{
int err;
spin_lock_irq(&efw->lock);
/* user land lock this */
if (efw->dev_lock_count < 0) {
err = -EBUSY;
goto end;
}
/* this is the first time */
if (efw->dev_lock_count++ == 0)
snd_efw_stream_lock_changed(efw);
err = 0;
end:
spin_unlock_irq(&efw->lock);
return err;
}
void snd_efw_stream_lock_release(struct snd_efw *efw)
{
spin_lock_irq(&efw->lock);
if (WARN_ON(efw->dev_lock_count <= 0))
goto end;
if (--efw->dev_lock_count == 0)
snd_efw_stream_lock_changed(efw);
end:
spin_unlock_irq(&efw->lock);
}
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...