Skip to content

Cloud-detected fuzz fixups - #7756

Merged
mwasko merged 4 commits into
thesofproject:mainfrom
andyross:fuzzfix1
Jun 9, 2023
Merged

mwasko merged 4 commits into
thesofproject:mainfrom
andyross:fuzzfix1

Conversation

@andyross

@andyross andyross commented Jun 6, 2023

Copy link
Copy Markdown
Contributor

A collection of minor fixes discovered by oss-fuzz over the last week. There's some guesswork involved here, as most of the crashes are lower down as other code follows e.g. bad pointers. I think the conditions here cover most (maybe all) of the failures I'm seeing reported. We'll see how that evolves, no doubt there will be more to come.

andyross added 4 commits June 6, 2023 08:43
The config block for the component gets provided externally and is
copied directly in using a byte count likewise provided by the host.
The use of memcpy_s() prevents overruns, but the error that was
detected was being reported via assert().  To fuzzing, that assertion
is a fatal error, when clearly this needs to be a runtime failure as
it's due to external input and not a local code bug.

Signed-off-by: Andy Ross <andyross@google.com>
This function is called based on external commands, and with an
arbitrary component ID that may not actually be a pipeline.  Check the
type before following garbage pointers and passing them down into call
trees.

Found via fuzzing.

Signed-off-by: Andy Ross <andyross@google.com>
These list heads in the comp_dev struct are not uniformly initialized
(grepping the source, the list_init() calls for these fields seem to
be spread around the source tree in individual components).  Fuzzing
is seeing nulls here, presumably because it's possible to reach
ipc_comp_free() in "unintended lifecycle" circumstances where they
weren't initialized.  Check the fields before crashing.

Signed-off-by: Andy Ross <andyross@google.com>
This handler was missing its type check and would follow comp_dev
union fields on components that might be buffers or pipelines, leading
to crashes downstream.

Found via fuzzing.

Signed-off-by: Andy Ross <andyross@google.com>

@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.

Thanks @andyross !

@cujomalainey

Copy link
Copy Markdown
Contributor

@mwasko fixes for cavs25-drop-stable as well as other stable branches

@mwasko
mwasko merged commit 65dc05a into thesofproject:main Jun 9, 2023
@kv2019i

kv2019i commented Jun 12, 2023 •

Copy link
Copy Markdown
Collaborator

@andyross @mwasko This introduced an rimage regression. Fixed in

@marc-hb

marc-hb commented Jun 12, 2023

Copy link
Copy Markdown
Collaborator

The accidental and unrelated rimage change was caught by Github check https://github.com/thesofproject/sof/actions/runs/5190604343/jobs/9357300136
(it's also clearly visible in the "Files changed" tab)

Please don't merge PRs with unexplained red crosses.

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.

5 participants