[lvc-project] [PATCH 6.1] wifi: iwlwifi: assume known PNVM power table size

[Dmitry Antipov]

On 1/29/25 1:48 PM, Johannes Berg wrote:

> I don't see that there's uninitialized use of 'len'. Maybe some static
> checkers aren't seeing through this, but the code is fine:
> 
> If iwl_pnvm_get_from_fs() is successful, then 'len' is initialized. If
> it fails, we goto skip_parse.
> 
> There, if trans->reduce_power_loaded is false, 'len' again is either
> initialized or we go to skip_reduce_power and never use 'len'.
> 
> If trans->reduce_power_loaded is true, then we get to
> iwl_trans_pcie_ctx_info_gen3_set_reduce_power() which doesn't use 'len'
> in this case.

Hm. As of 6.1.127, what I'm seeing is ('cat -n' annotated by me):

    258	int iwl_pnvm_load(struct iwl_trans *trans,
    259			  struct iwl_notif_wait_data *notif_wait)
    260	{
    261		u8 *data;
    262		size_t len;                                                     /* uninitialized */
    263		struct pnvm_sku_package *package;
    264		struct iwl_notification_wait pnvm_wait;
    265		static const u16 ntf_cmds[] = { WIDE_ID(REGULATORY_AND_NVM_GROUP,
    266							PNVM_INIT_COMPLETE_NTFY) };
    267		int ret;
    268	
    269		/* if the SKU_ID is empty, there's nothing to do */
    270		if (!trans->sku_id[0] && !trans->sku_id[1] && !trans->sku_id[2])
    271			return 0;
    272	
    273		/*
    274		 * If we already loaded (or tried to load) it before, we just
    275		 * need to set it again.
    276		 */
    277		if (trans->pnvm_loaded) {
    278			ret = iwl_trans_set_pnvm(trans, NULL, 0);
    279			if (ret)
    280				return ret;
    281			goto skip_parse;                                        /* taking goto */
    282		}
    283	
    284		/* First attempt to get the PNVM from BIOS */
    285		package = iwl_uefi_get_pnvm(trans, &len);
    286		if (!IS_ERR_OR_NULL(package)) {
    287			if (len >= sizeof(*package)) {
    288				/* we need only the data */
    289				len -= sizeof(*package);
    290				data = kmemdup(package->data, len, GFP_KERNEL);
    291			} else {
    292				data = NULL;
    293			}
    294	
    295			/* free package regardless of whether kmemdup succeeded */
    296			kfree(package);
    297	
    298			if (data)
    299				goto parse;
    300		}
    301	
    302		/* If it's not available, try from the filesystem */
    303		ret = iwl_pnvm_get_from_fs(trans, &data, &len);
    304		if (ret) {
    305			/*
    306			 * Pretend we've loaded it - at least we've tried and
    307			 * couldn't load it at all, so there's no point in
    308			 * trying again over and over.
    309			 */
    310			trans->pnvm_loaded = true;
    311	
    312			goto skip_parse;
    313		}
    314	
    315	parse:
    316		iwl_pnvm_parse(trans, data, len);
    317	
    318		kfree(data);
    319	
    320	skip_parse:
    321		data = NULL;
    322		/* now try to get the reduce power table, if not loaded yet */
    323		if (!trans->reduce_power_loaded) {                              /* the condition is false */
    324			data = iwl_uefi_get_reduced_power(trans, &len);
    325			if (IS_ERR_OR_NULL(data)) {
    326				/*
    327				 * Pretend we've loaded it - at least we've tried and
    328				 * couldn't load it at all, so there's no point in
    329				 * trying again over and over.
    330				 */
    331				trans->reduce_power_loaded = true;
    332	
    333				goto skip_reduce_power;
    334			}
    335		}
    336	
    337		ret = iwl_trans_set_reduce_power(trans, data, len);             /* len - ? */
    338		if (ret)
    339			IWL_DEBUG_FW(trans,
    340				     "Failed to set reduce power table %d\n",
    341				     ret);
    342		kfree(data);

Am I missing something?

Dmitry