Skip to content

Tplg parser security fixes#10896

Open
jsarha wants to merge 3 commits into
thesofproject:mainfrom
jsarha:tplg_parser_security_fixes
Open

Tplg parser security fixes#10896
jsarha wants to merge 3 commits into
thesofproject:mainfrom
jsarha:tplg_parser_security_fixes

Conversation

@jsarha

@jsarha jsarha commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Security fixes for development toos.

Jyri Sarha added 3 commits June 12, 2026 14:13
Add tplg_check_bounds() macro that validates ctx->tplg_offset + advance
does not exceed ctx->tplg_size before advancing the offset. Apply this
check in all topology object reader macros: tplg_get_hdr,
tplg_skip_hdr_payload, tplg_get_object, tplg_get_object_priv,
tplg_get_widget, tplg_get_graph, and tplg_get_pcm.

Without these checks, a crafted .tplg file with malicious payload_size
or priv.size values can drive the offset past the end of the mapped
topology data, causing out-of-bounds reads in all subsequent object
parsing.

Signed-off-by: Jyri Sarha <jyri.sarha@intel.com>
Replace unbounded strcat() calls with snprintf() that tracks remaining
buffer capacity. Add a pipeline_string_size parameter to
tplg_create_graph() so the function knows the buffer limit.

Without this fix, a crafted topology file with many or long graph
element names can overflow the caller's fixed 256-byte pipeline_string
buffer via repeated strcat(), corrupting the host stack.

Signed-off-by: Jyri Sarha <jyri.sarha@intel.com>
…erflow

Add a sanity check in process_sync() to reject packets with
data_size_bytes exceeding 16 MiB before performing the
data_size_bytes + sizeof(uint64_t) addition used for realloc sizing.

Without this check, a crafted probe dump with data_size_bytes near
UINT32_MAX wraps the realloc size to a small value, then the subsequent
data copy writes data_size_bytes into the undersized buffer.

Signed-off-by: Jyri Sarha <jyri.sarha@intel.com>
Copilot AI review requested due to automatic review settings June 12, 2026 11:37

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

Note

Copilot was unable to run its full agentic suite in this review.

Adds defensive bounds checking and safer string formatting to harden the topology parser and related tooling against malformed/untrusted inputs.

Changes:

  • Introduces a topology buffer bounds-check macro and applies it to multiple topology accessors.
  • Replaces unsafe strcat() concatenation with size-bounded snprintf() in graph pipeline-string construction.
  • Adds a sanity limit for probe packet data sizes to avoid oversized allocations/copies.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
tools/tplg_parser/include/tplg_parser/topology.h Adds bounds checking helpers and applies them to topology object getters; extends tplg_create_graph() API with buffer size.
tools/tplg_parser/graph.c Uses snprintf() with remaining capacity to avoid strcat() overflows when building the pipeline string.
tools/testbench/topology_ipc3.c Updates caller to pass pipeline_string capacity to tplg_create_graph().
tools/probes/probes_demux.c Adds a maximum allowed probe packet data size and rejects oversized packets.

Comment on lines 221 to 225
#define tplg_skip_hdr_payload(ctx) \
({struct snd_soc_tplg_hdr *ptr; \
tplg_check_bounds(ctx, hdr->payload_size); \
ptr = (struct snd_soc_tplg_hdr *)(ctx->tplg_base + ctx->tplg_offset); \
ctx->tplg_offset += hdr->payload_size; (void *)ptr; })
Comment on lines +199 to +208
#define tplg_check_bounds(ctx, advance) \
do { \
if ((long)(advance) < 0 || \
ctx->tplg_offset + (long)(advance) > (long)ctx->tplg_size) { \
printf("%s %d topology offset %ld + %ld > size %zu\n", \
__func__, __LINE__, ctx->tplg_offset, \
(long)(advance), ctx->tplg_size); \
assert(0); \
} \
} while (0)
Comment thread tools/tplg_parser/graph.c
Comment on lines +69 to +83
size_t cur_len = strlen(pipeline_string);
size_t remaining = pipeline_string_size > cur_len ?
pipeline_string_size - cur_len : 0;
int written;

if (route_num == (count - 1))
written = snprintf(pipeline_string + cur_len, remaining,
"%s->%s\n", graph_elem->source,
graph_elem->sink);
else
written = snprintf(pipeline_string + cur_len, remaining,
"%s->", graph_elem->source);

if (written < 0 || (size_t)written >= remaining)
fprintf(stderr, "warning: pipeline string truncated\n");
#define APP_NAME "sof-probes"

#define PACKET_MAX_SIZE 4096 /**< Size limit for probe data packet */
#define PACKET_DATA_SIZE_MAX (16 * 1024 * 1024) /**< Sanity limit for packet data size */
Comment on lines +198 to +202
if (p->packet->data_size_bytes > PACKET_DATA_SIZE_MAX) {
fprintf(stderr, "error: packet data size %u exceeds maximum %u\n",
p->packet->data_size_bytes, PACKET_DATA_SIZE_MAX);
return -EINVAL;
}
ctx->tplg_offset + (long)(advance) > (long)ctx->tplg_size) { \
printf("%s %d topology offset %ld + %ld > size %zu\n", \
__func__, __LINE__, ctx->tplg_offset, \
(long)(advance), ctx->tplg_size); \

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.

ctx should be in parentheses

({void *ptr; ptr = ctx->tplg_base + ctx->tplg_offset; \
({void *ptr; \
tplg_check_bounds(ctx, sizeof(*(obj)) + (priv_size)); \
ptr = ctx->tplg_base + ctx->tplg_offset; \

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.

same in these macros and below too

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.

3 participants