[Sound-open-firmware] [PATCH] ASoC: sof: Fix oops due to trace_sleep use before init
Access happens on the following path:
1) sof_ipc_tx_message => tx_wait_done => snd_sof_trace_notify_for_error => wake_up(&sdev->trace_sleep)
where sdev->trace_sleep is not yet initialized.
While at it also move initialization of sdev->host_offset before starting working with sdev struct.
Signed-off-by: Daniel Baluta daniel.baluta@gmail.com --- sound/soc/sof/trace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index 744d75b..2772cbee 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -245,6 +245,9 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) params.buffer.offset = 0; params.buffer.pages = sdev->dma_trace_pages;
+ init_waitqueue_head(&sdev->trace_sleep); + sdev->host_offset = 0; + /* send IPC to the DSP */ ret = sof_ipc_tx_message(sdev->ipc, params.hdr.cmd, ¶ms, sizeof(params), @@ -255,8 +258,6 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) goto table_err; }
- init_waitqueue_head(&sdev->trace_sleep); - sdev->host_offset = 0; return 0;
table_err:
Thanks Daniel for the patch. I'd need Liam and Yan to chime in since the error handling would be a bit different from what it is today, see below.
On 3/19/18 3:47 PM, Daniel Baluta wrote:
Access happens on the following path:
- sof_ipc_tx_message => tx_wait_done => snd_sof_trace_notify_for_error => wake_up(&sdev->trace_sleep)
where sdev->trace_sleep is not yet initialized.
While at it also move initialization of sdev->host_offset before starting working with sdev struct.
Signed-off-by: Daniel Baluta daniel.baluta@gmail.com
sound/soc/sof/trace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index 744d75b..2772cbee 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -245,6 +245,9 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) params.buffer.offset = 0; params.buffer.pages = sdev->dma_trace_pages;
- init_waitqueue_head(&sdev->trace_sleep);
- sdev->host_offset = 0;
- /* send IPC to the DSP */ ret = sof_ipc_tx_message(sdev->ipc, params.hdr.cmd, ¶ms, sizeof(params),
@@ -255,8 +258,6 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) goto table_err;
if for some reason this IPC fails, then we end-end with the trace_sleep and host_offset initialized. Is this ok?
}
init_waitqueue_head(&sdev->trace_sleep);
sdev->host_offset = 0; return 0;
table_err:
On Mon, 2018-03-19 at 16:28 -0700, Pierre-Louis Bossart wrote:
params.hdr.cmd, ¶ms, sizeof(params),@@ -255,8 +258,6 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) goto table_err;
if for some reason this IPC fails, then we end-end with the trace_sleep and host_offset initialized. Is this ok?
Yes.
Liam
Applied to topic/sof-v4.14
On 3/19/18 3:47 PM, Daniel Baluta wrote:
Access happens on the following path:
- sof_ipc_tx_message => tx_wait_done => snd_sof_trace_notify_for_error => wake_up(&sdev->trace_sleep)
where sdev->trace_sleep is not yet initialized.
While at it also move initialization of sdev->host_offset before starting working with sdev struct.
Signed-off-by: Daniel Baluta daniel.baluta@gmail.com
sound/soc/sof/trace.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/trace.c b/sound/soc/sof/trace.c index 744d75b..2772cbee 100644 --- a/sound/soc/sof/trace.c +++ b/sound/soc/sof/trace.c @@ -245,6 +245,9 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) params.buffer.offset = 0; params.buffer.pages = sdev->dma_trace_pages;
- init_waitqueue_head(&sdev->trace_sleep);
- sdev->host_offset = 0;
- /* send IPC to the DSP */ ret = sof_ipc_tx_message(sdev->ipc, params.hdr.cmd, ¶ms, sizeof(params),
@@ -255,8 +258,6 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev) goto table_err; }
init_waitqueue_head(&sdev->trace_sleep);
sdev->host_offset = 0; return 0;
table_err:
participants (3)
-
Daniel Baluta -
Liam Girdwood -
Pierre-Louis Bossart