Skip to content

probes: bound sample-rate index to the table size#10884

Open
lgirdwood wants to merge 1 commit into
thesofproject:mainfrom
lgirdwood:fix-probes
Open

probes: bound sample-rate index to the table size#10884
lgirdwood wants to merge 1 commit into
thesofproject:mainfrom
lgirdwood:fix-probes

Conversation

@lgirdwood

Copy link
Copy Markdown
Member

The four-bit sample-rate field could select an index past the rate table, which has fewer entries. Bound the index against the table size.

Copilot AI review requested due to automatic review settings June 11, 2026 15:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens sof-probes WAV header generation by preventing out-of-bounds access when decoding the 4-bit sample-rate index from the probe format field.

Changes:

  • Adds bounds checking for the sample-rate table index derived from the probe format field.
  • Introduces a local rate_idx variable and conditional selection to avoid reading past sample_rate[].

Comment thread tools/probes/probes_demux.c Outdated
Comment on lines +132 to +136
uint32_t rate_idx = (format & PROBE_MASK_SAMPLE_RATE) >> PROBE_SHIFT_SAMPLE_RATE;

p->files[i].header.fmt.sample_rate =
rate_idx < sizeof(sample_rate) / sizeof(sample_rate[0]) ?
sample_rate[rate_idx] : 0;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — an out-of-range index now falls back to a valid 48000 Hz rate (per lyakh's suggestion) rather than 0, so the WAV header stays usable, and the code genuinely clamps now.

@kv2019i kv2019i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message is off for this as well. And also copilot comment is good, better to set default to a valid rate.

@lgirdwood

Copy link
Copy Markdown
Member Author

Commit message is off for this as well. And also copilot comment is good, better to set default to a valid rate.

We need a CLAUDE.md here, I'm wondering if we have single workflows and just use soft links with each agents name.

lyakh
lyakh previously approved these changes Jun 12, 2026
Comment thread tools/probes/probes_demux.c Outdated
if (rate_idx >= sizeof(sample_rate) / sizeof(sample_rate[0]))
rate_idx = PROBE_DEFAULT_RATE_IDX;

p->files[i].header.fmt.sample_rate = sample_rate[rate_idx];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fact I think this would be better:

if (rate_idx >= sizeof(sample_rate) / sizeof(sample_rate[0]))
    p->files[i].header.fmt.sample_rate = 48000;
else
    p->files[i].header.fmt.sample_rate = sample_rate[rate_idx];

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adopted — thanks, that's cleaner. Done: out-of-range now sets the rate directly to 48000 instead of going through a default-index macro. Force-pushed.

@lyakh lyakh dismissed their stale review June 12, 2026 09:56

sorry, changed my mind, I think there's a simple nice way to avoid the manually managed default rate index

@abonislawski

Copy link
Copy Markdown
Member

We need a CLAUDE.md here, I'm wondering if we have single workflows and just use soft links with each agents name.

Every agent will support our single AGENTS.md except ...Claude. They are probably the last ones waiting to adapt the standard md name.

The 4-bit sample-rate field can select an index past the end of the
sample_rate[] table, which has fewer than 16 entries, reading out of
bounds. Clamp an out-of-range index to a valid default rate (48000 Hz)
so the read stays in bounds and the generated WAV header keeps a usable
sample rate.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants