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
