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