ipc4: large_config: log rejected data_off_size validation failures#10934
ipc4: large_config: log rejected data_off_size validation failures#10934jsarha wants to merge 1 commit into
Conversation
jsarha
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR improves diagnosability of IPC4 MOD_LARGE_CONFIG_SET rejections by adding debug traces when data_off_size fails existing bounds checks in ipc4_set_vendor_config_module_instance() (no change to validation logic).
Changes:
- Add
tr_dbg()logging whendata_off_sizeexceedsMAILBOX_HOSTBOX_SIZE. - Add
tr_dbg()logging wheninit_blockrequests provide adata_off_sizesmaller thansizeof(struct sof_tlv).
| tr_dbg(&ipc_tr, "data_off_size greater than mailbox %u > %u", | ||
| data_off_size, MAILBOX_HOSTBOX_SIZE); |
There was a problem hiding this comment.
Why on earth copilot wants to cast literal macro to size_t and then use %zu ? Double space fixed.
There was a problem hiding this comment.
@jsarha was this giving a warning for native sim/fuzzing builds. I dont see it giving an issue today on 64bit since its sizes.
| tr_dbg(&ipc_tr, "init_block data_off_size too small %u < %u", | ||
| data_off_size, sizeof(struct sof_tlv)); |
jsarha
force-pushed
the
ipc_user_handler_security_fixes_followup
branch
2 times, most recently
from
b91d399 to
d4c5415
Compare
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Comment on the changes inline. -1 due to wrong sign-off...
|
|
||
| /* Validate host-controlled payload size before any use or arithmetic. */ | ||
| if (data_off_size > MAILBOX_HOSTBOX_SIZE) | ||
| if (data_off_size > MAILBOX_HOSTBOX_SIZE) { |
There was a problem hiding this comment.
tr_dbg() logs are disabled by default, so not sure of the benefit., If we want to leave these in production binaries, we need to use err or warn. And not 100% sure this is worth the binary space to have the logs for errors like these that will only occur if completely out-of-spec IPC messages are sent by host.
There was a problem hiding this comment.
Damn. Copilot stole my commit. I wrote the changes and only asked copilot to write the commit message and it stole all the credit.
The data_off_size bounds checks in ipc4_set_vendor_config_module_instance() silently returned IPC4_INVALID_CONFIG_DATA_STRUCT, giving no clue why a MOD_LARGE_CONFIG_SET request was rejected. This makes diagnosing malformed or malicious topologies/host requests harder. Add tr_dbg() traces to both rejection paths reporting the offending data_off_size together with the limit it violated (the mailbox size for the upper bound, and sizeof(struct sof_tlv) for the init_block lower bound). No functional change to the validation itself. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
ipc_user_handler_security_fixes_followup
branch
from
d4c5415 to
a548c0c
Compare
The data_off_size bounds checks in ipc4_set_vendor_config_module_instance() silently returned IPC4_INVALID_CONFIG_DATA_STRUCT, giving no clue why a MOD_LARGE_CONFIG_SET request was rejected. This makes diagnosing malformed or malicious topologies/host requests harder.
Add tr_dbg() traces to both rejection paths reporting the offending data_off_size together with the limit it violated (the mailbox size for the upper bound, and sizeof(struct sof_tlv) for the init_block lower bound). No functional change to the validation itself.