Skip to content

ipc-helper.c: reject invalid SOF_MEM_CAPS_* bits #8850

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.

Sign up for GitHub

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

Jump to bottom
Merged
kv2019i merged 1 commit into from
Mar 5, 2024
Merged

ipc-helper.c: reject invalid SOF_MEM_CAPS_* bits #8850

kv2019i merged 1 commit into from
Mar 5, 2024

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Feb 10, 2024

Fixes lack of SOF_MEM_CAPS_* input validation found in #8832

@marc-hb

This comment was marked as outdated.

Sign in to view
@marc-hb

This comment was marked as outdated.

Sign in to view
@marc-hb marc-hb mentioned this pull request Feb 10, 2024
Closed
lyakh
lyakh reviewed Feb 12, 2024
View reviewed changes
lgirdwood
lgirdwood reviewed Feb 19, 2024
View reviewed changes
@marc-hb marc-hb marked this pull request as ready for review February 27, 2024 20:21
@marc-hb
Copy link
Collaborator Author

marc-hb commented Feb 27, 2024

Unrelated "signal overflow!" failure in https://sof-ci.01.org/sofpr/PR8850/build2957/devicetest/index.html?model=MTLP_RVP_NOCODEC&testcase=check-alsabat-headset-playback-997 on sh-mtlp-rvp-nocodec-06. Everything else green.

1 unrelated suspend/resume failure in https://sof-ci.01.org/sofpr/PR8850/build2958/devicetest/index.html, everything else green.

1 unrelated failure in https://sof-ci.01.org/sof-pr-viewer/#/build/PR8850/build13640869, every other test is green.

12:53:09,629 INFO  - tests/avs/fw_01_base_fw_modules/test_13_updwmix.py::TestUpdwmix::test_01_13_updownmixer_bat_scope[48000Hz_32in32bit_4ch-48000Hz_32in32bit_2ch1]
12:53:09,649 INFO  - [gw0] [100%] FAILED tests/avs/fw_01_base_fw_modules/test_13_updwmix.py::TestUpdwmix::test_01_13_updownmixer_bat_scope[48000Hz_32in32bit_4ch-48000Hz_32in32bit_2ch1]
12:53:09,650 INFO  -
12:53:09,650 INFO  - =================================== FAILURES ===================================
12:53:09,650 INFO  - _ TestUpdwmix.test_01_13_updownmixer_bat_scope[48000Hz_32in32bit_4ch-48000Hz_32in32bit_2ch1] _

lgirdwood
lgirdwood reviewed Feb 28, 2024
View reviewed changes
src/include/ipc/topology.h Outdated
#define SOF_MEM_CAPS_EXEC BIT(7) /**< executable */
#define SOF_MEM_CAPS_L3 BIT(8) /**< L3 memory */

#define SOF_MEM_CAPS_MAX_BIT BIT(8) /**< Used for input validation */
Copy link
Member

@lgirdwood lgirdwood Feb 28, 2024

Choose a reason for hiding this comment

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

Since this is ABI, its better we make it obvious via an inline comment that MAX_BIT needs to be updated for any new MEM_CAPS addition. I would also define MAX_BIT as CAPS_L3 rather than BIT(8) as this makes it even clearer to anyone reading the code.

Copy link
Collaborator Author

@marc-hb marc-hb Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we make it obvious via an inline comment that MAX_BIT needs to be updated for any new MEM_CAPS addition.

Comment added.

I would also define MAX_BIT as CAPS_L3 rather than BIT(8) as this makes it even clearer to anyone reading the code.

I can't do that because I realized v2 12aff94 was buggy, apologies. In v2 I made _MAX equal to the last last valid bit but that is wrong, it would incorrectly reject _MAX + some other BIT. The tests passed anyway because we don't seem to test L3_HEAP.

The comparison must be with the first invalid bit. I confused myself with the name "_MAX", it is a bad name. I just replaced it with _LOWEST_INVALID = max << 1

@marc-hb marc-hb marked this pull request as draft February 28, 2024 18:10
@marc-hb
ipc-helper.c: reject invalid SOF_MEM_CAPS_* bits
10cba1f 
Fixes lack of SOF_MEM_CAPS_* input validation found in thesofproject#8832

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb marked this pull request as ready for review February 28, 2024 19:29
@marc-hb
Copy link
Collaborator Author

marc-hb commented Feb 28, 2024

Only suspend/resume failures in CAVS https://sof-ci.01.org/sofpr/PR8850/build2989/devicetest/index.html

ACE https://sof-ci.01.org/sofpr/PR8850/build2988/devicetest/index.html is all green.

@lgirdwood lgirdwood added this to the v2.9 milestone Feb 29, 2024
@kv2019i kv2019i merged commit 4f59ee8 into thesofproject:main Mar 5, 2024
@marc-hb marc-hb deleted the mem-caps-max branch March 7, 2024 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lyakh lyakh lyakh left review comments

@lgirdwood lgirdwood lgirdwood approved these changes

@kv2019i kv2019i kv2019i approved these changes

@bardliao bardliao Awaiting requested review from bardliao bardliao is a code owner

@marcinszkudlinski marcinszkudlinski Awaiting requested review from marcinszkudlinski marcinszkudlinski is a code owner

@pblaszko pblaszko Awaiting requested review from pblaszko pblaszko is a code owner

@RanderWang RanderWang Awaiting requested review from RanderWang RanderWang is a code owner

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@jxstelter jxstelter Awaiting requested review from jxstelter

@ujfalusi ujfalusi Awaiting requested review from ujfalusi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.9

Development

Successfully merging this pull request may close these issues.

4 participants

@marc-hb @lyakh @lgirdwood @kv2019i