Skip to content

Conversation

@iuliana-prodan
Copy link
Contributor

@iuliana-prodan iuliana-prodan commented Oct 10, 2023

Revert "module_adapter: avoid module init crash in case of ipc data invalid"

This reverts 'commit e847c8b ("module_adapter: avoid module
init crash in case of ipc data invalid")'.

No data is not an invalid case for mixer, for example.

Fixes: #8265
Signed-off-by: Iuliana Prodan iuliana.prodan@nxp.com

dst->init_data = dst->data;
} else if (size == 0) {
/* valid case for mixer */
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is the case, do we still need keep line 85 - 87?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@btian1 true, size can't be negative. So maybe we just do a full revert.

It does open questions how to address the original commit e847c8b . Do we need a SRC specific patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree @kv2019i , I'll do a full revert of this e847c8b, but the revert doesn't apply cleanly now because of the ipc3/ipc4 split.

I've kept the check for negative size to not break something on Intel targets - even though the explanation for adding that commit is limited.

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.

I'd suggest a revert and then do a follow-up to make a new fix to avoiding module init crashes (if a generic module level solution is possible, not sure it is).

dst->init_data = dst->data;
} else if (size == 0) {
/* valid case for mixer */
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@btian1 true, size can't be negative. So maybe we just do a full revert.

It does open questions how to address the original commit e847c8b . Do we need a SRC specific patch?

…nvalid"

This reverts 'commit e847c8b ("module_adapter: avoid module
init crash in case of ipc data invalid")'.

No data is not an invalid case for mixer, for example.

Fixes: thesofproject#8265
Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
@iuliana-prodan iuliana-prodan changed the title ipc3: module adapter: do not crash in case of no data Revert "module_adapter: avoid module init crash in case of ipc data invalid" Oct 12, 2023
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.

Looks good. @btian1 can you do a follow-up?

@btian1
Copy link
Contributor

btian1 commented Oct 13, 2023

an you do a follow-up?

Looks good. @btian1 can you do a follow-up?

e847c8b, for me this is a safeguard to avoid module init crash, I remember there is another patch, I forgot the commit name to fix similar issue.
let's merge this first, then check if have any further issues.

@dbaluta dbaluta merged commit bb8d6ba into thesofproject:main Oct 13, 2023
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.

[BUG] Unable to load tplg with mixer or DRC component

4 participants