From d181485a30a014f9cb9a92b0b2334b4838e8ad6c Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 7 Oct 2026 14:31:18 +0200 Subject: [PATCH 1/3] pipeline: clamp latency accumulation against unsigned underflow pipeline_get_dai_comp_latency() accumulated the buffered block count as *latency += input_data / ibs - output_data / obs; comp_get_total_data_processed() returns uint64_t and ibs/obs are uint32_t, so both divisions and the subtraction are evaluated as unsigned 64-bit with no ordering guarantee between the two terms. When the sink has processed more blocks than the source the expression wraps to a value close to 2^64 and is then truncated into the uint32_t *latency as a huge block count. The sink leading the source is not an error case: it is exactly the "dai is enabled before host since they are in different pipelines" scenario described by the copier code that consumes this value. The resulting latency is multiplied by the period size in copier_comp_trigger() and published to the host driver as stream_start_offset, so the host computes its stream position from a bogus offset. A buffered block count cannot be negative, so only accumulate when the source leads the sink. Assisted-by: Copilot:claude-opus-5 Signed-off-by: Tomasz Leman --- src/audio/pipeline/pipeline-graph.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index f6f83a0e8dc6..b4e5d5cb4ff5 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -699,9 +699,20 @@ struct comp_dev *pipeline_get_dai_comp_latency(uint32_t pipeline_id, uint32_t *l if (ret < 0) return NULL; - if (input_data && output_data && input_base_cfg.ibs && output_base_cfg.obs) - *latency += input_data / input_base_cfg.ibs - - output_data / output_base_cfg.obs; + if (input_data && output_data && input_base_cfg.ibs && output_base_cfg.obs) { + uint64_t in_blocks = input_data / input_base_cfg.ibs; + uint64_t out_blocks = output_data / output_base_cfg.obs; + + /* The latency is a count of blocks buffered between the + * source and the sink and cannot be negative. The sink may + * legitimately lead the source, e.g. when the dai is started + * before the host because they are in different pipelines, + * so clamp instead of letting the unsigned subtraction wrap + * into a huge value. + */ + if (in_blocks > out_blocks) + *latency += in_blocks - out_blocks; + } /* If the component doesn't have a sink buffer, it can be a dai. */ if (list_is_empty(&ipc_sink->cd->bsink_list)) From 7641d8bad0cfc299cea8a846a1e850676c26ea48 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Wed, 7 Oct 2026 13:50:34 +0200 Subject: [PATCH 2/3] module: copier: compute stream start offset at 64-bit width latency and audio_stream_period_bytes() are both 32 bit, so the product was computed in 32-bit arithmetic before being added to the 64-bit stream_start_offset reported to the host driver. With well formed values the product stays around 10^6 bytes, far below the 32-bit limit: an overflow would require a latency of roughly 2.8 million blocks, i.e. tens of minutes of buffered audio. The only way to reach such a value was the unsigned underflow in pipeline_get_dai_comp_latency() fixed in the preceding commit, and a cast alone would not have helped there. This change is therefore hardening, not a fix for a reachable overflow. Cast latency to uint64_t on both the START and RELEASE paths so the outer multiplication is carried out at the width of the destination. Reported by CodeQL cpp/integer-multiplication-cast-to-long. Assisted-by: Copilot:claude-opus-5 codeql Signed-off-by: Tomasz Leman --- src/audio/copier/copier.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/audio/copier/copier.c b/src/audio/copier/copier.c index 6007dd01642c..7fddbc0a034c 100644 --- a/src/audio/copier/copier.c +++ b/src/audio/copier/copier.c @@ -494,8 +494,8 @@ static int copier_comp_trigger(struct comp_dev *dev, int cmd) } buffer = comp_dev_get_first_data_producer(dai_copier); - pipe_reg.stream_start_offset = posn.dai_posn + - latency * audio_stream_period_bytes(&buffer->stream, dev->frames); + pipe_reg.stream_start_offset = posn.dai_posn + (uint64_t)latency * + audio_stream_period_bytes(&buffer->stream, dev->frames); pipe_reg.stream_end_offset = 0; mailbox_sw_regs_write(cd->pipeline_reg_offset, &pipe_reg, sizeof(pipe_reg)); } else if (cmd == COMP_TRIGGER_PAUSE) { @@ -518,7 +518,7 @@ static int copier_comp_trigger(struct comp_dev *dev, int cmd) } buffer = comp_dev_get_first_data_producer(dai_copier); - pipe_reg.stream_start_offset += latency * + pipe_reg.stream_start_offset += (uint64_t)latency * audio_stream_period_bytes(&buffer->stream, dev->frames); mailbox_sw_regs_write(cd->pipeline_reg_offset, &pipe_reg.stream_start_offset, sizeof(pipe_reg.stream_start_offset)); From 17272cf8177acae65554a36204d2f03815d83679 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Thu, 8 Oct 2026 15:00:22 +0200 Subject: [PATCH 3/3] module: copier: ignore out of order dai position on release stream_end_offset is sampled from the dai position when the pipeline is paused and is read back from the host visible mailbox on release. The release path subtracts it from the current dai position with an unsigned 64-bit subtraction and no ordering check, so a stale or out of order value wraps into a huge number and corrupts the stream start offset reported to the host driver. The host computes the stream position as position = DMA counter - stream_start_offset so a corrupted offset is directly visible as a bogus position. Accumulate only when the current position is actually ahead of the saved end offset, mirroring the clamp applied to the latency accumulation in pipeline_get_dai_comp_latency(). Assisted-by: Copilot:claude-opus-5 Signed-off-by: Tomasz Leman --- src/audio/copier/copier.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/audio/copier/copier.c b/src/audio/copier/copier.c index 7fddbc0a034c..4ec85cb64fba 100644 --- a/src/audio/copier/copier.c +++ b/src/audio/copier/copier.c @@ -510,7 +510,15 @@ static int copier_comp_trigger(struct comp_dev *dev, int cmd) pipe_reg.stream_start_offset = mailbox_sw_reg_read64(cd->pipeline_reg_offset); pipe_reg.stream_end_offset = mailbox_sw_reg_read64(cd->pipeline_reg_offset + sizeof(pipe_reg.stream_start_offset)); - pipe_reg.stream_start_offset += posn.dai_posn - pipe_reg.stream_end_offset; + + /* stream_end_offset was sampled from the dai position when the pipeline + * was paused, so the current position should be ahead of it. The value + * is read back from the host visible mailbox and cannot be trusted: a + * stale or out of order one would make the unsigned subtraction wrap + * and corrupt the offset reported to the host driver. + */ + if (posn.dai_posn > pipe_reg.stream_end_offset) + pipe_reg.stream_start_offset += posn.dai_posn - pipe_reg.stream_end_offset; if (list_is_empty(&dai_copier->bsource_list)) { comp_err(dev, "No source buffer bound to dai_copier");