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

ipc3: add subsize check #9374

Merged
merged 1 commit into from
Aug 28, 2024
Merged

ipc3: add subsize check #9374

merged 1 commit into from
Aug 28, 2024

Conversation

cujomalainey
Copy link
Member

I missed a case where the process size should be less than the ext size. Fuzzer found the loophole and had some fun.

@cujomalainey
Copy link
Member Author

Hmm looks like the CI found some issues. Going to check if this is a test issue or check bug.

@cujomalainey
Copy link
Member Author

@singalsu @RanderWang the FIR init config for cmocka is invalid. 146 * 4 = 584. This violates IPC3 SOF_IPC_MSG_MAX_SIZE which is defined as 384. Can you fix this test case?

@cujomalainey
Copy link
Member Author

cujomalainey commented Aug 20, 2024

Same story for the testbench, config of 616 being passed into the priv config for FIR.

@lgirdwood
Copy link
Member

@singalsu @RanderWang the FIR init config for cmocka is invalid. 146 * 4 = 584. This violates IPC3 SOF_IPC_MSG_MAX_SIZE which is defined as 384. Can you fix this test case?

@singalsu are you able to fix the mock ? Thanks !!

@cujomalainey cujomalainey added the bug Something isn't working as expected label Aug 22, 2024
@singalsu
Copy link
Collaborator

@singalsu @RanderWang the FIR init config for cmocka is invalid. 146 * 4 = 584. This violates IPC3 SOF_IPC_MSG_MAX_SIZE which is defined as 384. Can you fix this test case?

@singalsu are you able to fix the mock ? Thanks !!

Yep, I will. Good catch @cujomalainey !

@singalsu
Copy link
Collaborator

@cujomalainey @lgirdwood The fix for FIR unit test is in #9391.

@cujomalainey
Copy link
Member Author

Moving back to draft to prevent merge until #9391 is resolved

@cujomalainey
Copy link
Member Author

Looks like #9402 fixes the failures. Thanks @singalsu

@cujomalainey cujomalainey marked this pull request as ready for review August 27, 2024 21:38
The ext size is counted as part of the ipc header size. The process size
and header size need to be less than the ipc max size.

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
@cujomalainey
Copy link
Member Author

Rebased on top of the submitted version of #9402 cmocka and testbench should come back clean

@cujomalainey
Copy link
Member Author

@wszypelt what is the story on CI?

@wszypelt
Copy link

@cujomalainey problem with one machine on CI, but fixed and from what I see tests are already on pass

@kv2019i kv2019i merged commit 94b3d2c into thesofproject:main Aug 28, 2024
46 of 47 checks passed
@cujomalainey cujomalainey deleted the size branch August 28, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants