Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud-detected fuzz fixups #7756

Merged
merged 4 commits into from
Jun 9, 2023
Merged

Cloud-detected fuzz fixups #7756

merged 4 commits into from
Jun 9, 2023

Conversation

andyross
Copy link
Contributor

@andyross andyross commented Jun 6, 2023

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>
Copy link
Collaborator

@kv2019i kv2019i left a comment

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
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
Copy link
Collaborator

kv2019i commented Jun 12, 2023

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

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 12, 2023

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