ASoC: SOF: debug: Do not create fw_lib_path file in case of IPC3 #4945
Conversation
|
@marc-hb are you good with this? |
sound/soc/sof/debug.c
Outdated
| debugfs_create_str("fw_lib_path", 0444, fw_profile, | ||
| (char **)&plat_data->fw_lib_prefix); | ||
| /* library path is not valid for IPC3 */ | ||
| if (plat_data->ipc_type != SOF_IPC_TYPE_3) |
There was a problem hiding this comment.
Why not test plat_data->fw_lib_prefix directly? Is it not initialized yet? If not, does this leave a short time window of vulnerability?
There was a problem hiding this comment.
The IPC-based detection will force us to add this path for new platforms, otherwise we get a kernel oops
If we test the fw_lib_prefix, we won't be able to detect a missing setting, unless someone looks in detail at the CI logs - not something we excel at, do we?
There was a problem hiding this comment.
- If
sof_select_ipc_and_paths()is invoked before this code, then to make sure each new platform definesfw_lib_paththen you should have some sort of assert here and not just rely on sof-test to detect this NULL maybe, maybe not. I mean something like this:
if (plat_data->ipc_type != SOF_IPC_TYPE_3) {
APPROPRIATE_NOT_NULL_KERNEL_ASSERT(fw_lib_prefix);
...
}- If
sof_select_ipc_and_paths()is NOT invoked before this code, then there is short time window with a vulnerability. Maybe some static analyzer will find it?
There was a problem hiding this comment.
then you should have some sort of assert here and not just rely on sof-test to detect this NULL maybe, maybe not.
Assert "here" or maybe rather in sof_pci_probe() or whichever function initializes snd_sof_pdata. That function should be in charge of not producing garbage platform data.
There was a problem hiding this comment.
I would leave the door open for vendors to not use libraries even with IPC4... The library loading taking this into account (handles fw_lib_prefix == NULL with dev_err() print).
It is still true that IPC3 does not even have support for this, so I would allocate a string "Not supported" and pass that in case of ipc_type != IPC3 && !fw_lib_path.
The firmware libraries are not supported by IPC3, the fw_lib_path is not a valid parameter and it is always NULL. Do not create the debugfs file for IPC3 at all as it is not applicable. With IPC4 some vendors/platforms might not support loadable libraries and the fw_lib_prefix is left to NULL to indicate this. Handle such case with allocating "Not supported" string. Fixes: 17f4041 ("ASoC: SOF: debug: show firmware/topology prefix/names") Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
ujfalusi
force-pushed
the
peter/sof/pr/debug_no_fw_lib_path_for_ipc3_01
branch
from
6b4ebd3 to
62bce67
Compare
|
Changes since v1:
|
5 participants
The firmware libraries are not supported by IPC3, the fw_lib_path is not a valid parameter and it is always NULL.
Fixes: 17f4041 ("ASoC: SOF: debug: show firmware/topology prefix/names")