Re: [PATCH v3 09/10] ASoC: Intel: avs: Refactor and fix init_config access

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 9/1/2026 12:30 AM, Mark Brown wrote:
> On Mon, Aug 31, 2026 at 06:42:32PM +0200, Cezary Rojewski wrote:
>> Existing code accesses enties found in ->init_configs array through
>> indexes that are part of ->config_ids array.  Those two are limited by:
>> ->num_init_configs and ->num_config_ids respectively.  Using ID larger
>> or equal to ->num_init_configs leads to out-of-bounds access:
> 
> ...
> 
>> Rather than adding another if-statement, refactor the code.  There is no
>> need to store the IDs, have a list of pointers to actual config-entries
>> instead.  As the verification of ->init_config entries does not differ from
>> verification of other types that are part of the topology.c file, simply
>> reuse the code.
> 
>>  static int avs_path_module_send_init_configs(struct avs_dev *adev, struct avs_path_module *mod)
> 
>> +       for (int i = 0; i < template->num_init_configs; i++) {
>> +               struct avs_tplg_init_config *config = template->init_configs[i];
>>                 size_t len = config->length;
> 
> This will dereference every init_configs entry...
> 
>> +	cfgs = devm_kcalloc(comp->card->dev, module->num_init_configs, sizeof(*cfgs), GFP_KERNEL);
>> +	if (!cfgs)
>> +		return -ENOMEM;
>> +
>> +	ret = parse_dictionary_entries(comp, tuples, block_size, cfgs, module->num_init_configs,
>> +				       sizeof(*cfgs), AVS_TKN_MOD_INIT_CONFIG_ID_U32,
>> +				       init_config_parsers, ARRAY_SIZE(init_config_parsers));
> 
> ...but IIRC this ignores things it doesn't understand so will leave NULL
> entries behind.  You'd presumably need malformed firwmare or something
> but still, we're parsing data from userspace.

Thank you for the review!  Yes, the parser will ignore unknown entries
but these are two separate dictionaries. TBH I'm not seeing the problem
but the technical background first:

struct avs_tplg holds the array of init_configs:
struct avs_tplg {
	(...)
	u32 num_init_configs;
	struct avs_tplg_init_config *init_configs;
};

That memory block gets allocated when the topology manifest is being loaded.

Now, struct avs_tplg_module holds the array of references to existing
entries in tplg->init_configs:

struct avs_tplg_module {
	(...)
	u32 num_init_configs;
	struct avs_tplg_init_config **init_configs;
};

tplg->num_init_configs != module->num_init_configs
tplg->init_configs != module->init_configs

The memory block gets allocated when the module template is being loaded
(DAI widget).  In contract to the manifest, these entries are just
references - parser avs_parse_init_config_ptr() is used (comes from
AVS_DEFINE_PTR_PARSER macro).  The parser checks whether the ID exists
in tplg->init_configs and then assigns the reference.

The goal behind this procedure: reduce the amount of memory allocated.
The manifest takes that burden and all other topology components just
reference its entries.  No information is duplicated.

As I'm very used to the driver and its files it's quite possible I'm not
seeing something very obvious.