module adapter: avoid module init crash in case of ipc data invalid

[btian1]

In ipc3 module creation, it is possible that ipc data is invalid or corrupted, in this case, module init may crash.
This patch is adding error handling to avoid crash.

[btian1]

@jsarha @cujomalainey ， since #7985 reverted #7830, how about this patch? it also try to exit with 0 size case.

[dbaluta]

@btian1 I would love to see a more detailed explanation and analysis of the problem this patch is trying to fix.

[btian1]

@btian1 I would love to see a more detailed explanation and analysis of the problem this patch is trying to fix.

there is a issue when src module is called, see #7830, when init_data is NULL, module need exit gracefully, however, seems #7830 have issues, and #7985 reverted, so I submit this try to resolve same issue

[dbaluta]

there is a issue when src module is called, see #7830, when init_data is NULL, module need exit gracefully, however, seems #7830 have issues, and #7985 reverted, so I submit this try to resolve same issue

You need to put the explanation in the commit message. When trying to understand the code people usually read commit messages / the code, they won't know about past PR.

[btian1]

there is a issue when src module is called, see #7830, when init_data is NULL, module need exit gracefully, however, seems #7830 have issues, and #7985 reverted, so I submit this try to resolve same issue

You need to put the explanation in the commit message. When trying to understand the code people usually read commit messages / the code, they won't know about past PR.

thanks, will do once move pr to open state, currently, it is still Draft, waiting for feedback.

[lgirdwood]

@jsarha can you help/comment here as you have the context.

[jsarha]

@jsarha can you help/comment here as you have the context.

I was thinking of similar kind of fix when doing #7830, but I was not sure if there could be some modules out there that did not need any init data. But since IPC3 should be legacy only and no new modules should appear there, I think this should be Ok if it passes our CI. Or is there some untested, but important module out there, that does not need any init data?

[btian1]

Thanks, @jsarha, could you post the origin issue link and the verify steps for origin issue? we can verify first and then decide next steps.

[jsarha]

Thanks, @jsarha, could you post the origin issue link and the verify steps for origin issue? we can verify first and then decide next steps.

It was a fuzzer issue, and I do not know how reproduce them, but I can copy-paste a backtrace. But before that, these are security issues that try to prevent malicious messages from crashing the FW. So any check that prevents an error that can happen (or is known to happen) should be good, as long as the check is not too strict and cause other errors.

Also, there is already a module specific fix for the case bellow here: #8004 , so there is no need to fix that particular case, but as a generic fix this may solve other issues too. Then again I guess our current standing is to do only the bare minimum for IPC3, so then again maybe this is not needed.

And then the backtrace: =================================================================
==2837==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000 (pc 0x0828be59 bp 0xf0afbe68 sp 0xf0afbe10 T7)
==2837==The signal is caused by a READ memory access.
==2837==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
#0 0x828be59 in src_rate_check sof/sof/src/audio/src/src.c:574:15
#1 0x8282b5d in module_init sof/sof/src/audio/module_adapter/module/generic.c:110:8
#2 0x827847f in module_adapter_new sof/sof/src/audio/module_adapter/module_adapter.c:134:8
#3 0x828b1fc in module_src_interface_shim_new sof/sof/src/audio/src/src.c:1131:1
#4 0x8210aad in comp_new sof/sof/src/ipc/ipc3/helper.c:333:9
#5 0x8214e26 in ipc_comp_new sof/sof/src/ipc/ipc3/helper.c:654:7
#6 0x82092f5 in ipc_glb_tplg_comp_new sof/sof/src/ipc/ipc3/handler.c:1298:8
#7 0x8208939 in ipc_cmd sof/sof/src/ipc/ipc3/handler.c:1680:9
#8 0x81cbed8 in ipc_platform_do_cmd sof/sof/src/platform/posix/ipc.c:162:2
#9 0x81d10db in ipc_do_cmd sof/sof/src/ipc/ipc-common.c:330:9
#10 0x81f87e9 in task_run sof/sof/zephyr/include/rtos/task.h:94:9
#11 0x81f8308 in edf_work_handler sof/sof/zephyr/edf_schedule.c:31:16
#12 0x82b4b32 in work_queue_main sof/zephyr/kernel/work.c:668:3
#13 0x8193ec2 in z_thread_entry sof/zephyr/lib/os/thread_entry.c:36:2
#14 0x815f639 in __asan::AsanThread::ThreadStart(unsigned long long) /src/llvm-project/compiler-rt/lib/asan/asan_thread.cpp:277:25

[jsarha]

And the most detailed explanation of what was the actual problem, can be found here (look neighboring comments too): #7830 (comment)

[btian1]

Thanks, @jsarha , #8004 only resolved src_init fuzzer issue, if there is another module have potential issues, then same issue will happen again.

If you check ipc4 part, dst->init_data will always have a valid assignment, ipc3 does not, this patch at least can resolve potential
module init issues without other risk, agree, @jsarha @dbaluta @lgirdwood @ranj063

[ranj063]

@btian1 you still need to explain the change better in the commit message. You're not trying to skip module init, but rather returning an error to not crash in case the IPC data is corrupted for processing modules that have no init data, right?

[cujomalainey]

Is no data a valid part of the spec? If yes then this is invalid

[kv2019i]

SOFCI TEST

[kv2019i]

Looks ready to go, just need a clean CI run.

[btian1]

SOFCI TEST

[kv2019i]

On DUT still unavailable, but rest is good https://sof-ci.01.org/sofpr/PR7992/build11414/devicetest/index.html (one system PM fail but these are known). Missing DUTs in https://sof-ci.01.org/sofpr/PR7992/build11415/devicetest/index.html as well, but the available ones pass. Rest of CI jobs are passing, so proceeding with merge.

[marc-hb]

Reverted in

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