Hi, Static analysis on linux-next wit Coverity has found a potential null pointer dereference in commit: commit 70732504c53b2d3aae2cebc457515a304672d5bb Author: Yongqiang Sun <yongqiang.sun@xxxxxxx> Date: Fri Feb 19 14:50:23 2021 -0500 drm/amd/display: Implement dmub trace event The analysis is as follows: 400 enum dmub_status dmub_srv_hw_init(struct dmub_srv *dmub, 401 const struct dmub_srv_hw_params *params) 402 { 403 struct dmub_fb *inst_fb = params->fb[DMUB_WINDOW_0_INST_CONST]; 404 struct dmub_fb *stack_fb = params->fb[DMUB_WINDOW_1_STACK]; 405 struct dmub_fb *data_fb = params->fb[DMUB_WINDOW_2_BSS_DATA]; 406 struct dmub_fb *bios_fb = params->fb[DMUB_WINDOW_3_VBIOS]; 407 struct dmub_fb *mail_fb = params->fb[DMUB_WINDOW_4_MAILBOX]; 408 struct dmub_fb *tracebuff_fb = params->fb[DMUB_WINDOW_5_TRACEBUFF]; 409 struct dmub_fb *fw_state_fb = params->fb[DMUB_WINDOW_6_FW_STATE]; 410 struct dmub_fb *scratch_mem_fb = params->fb[DMUB_WINDOW_7_SCRATCH_MEM]; 411 412 struct dmub_rb_init_params rb_params, outbox0_rb_params; 413 struct dmub_window cw0, cw1, cw2, cw3, cw4, cw5, cw6; 414 struct dmub_region inbox1, outbox1, outbox0; 415 1. Condition !dmub->sw_init, taking false branch. 416 if (!dmub->sw_init) 417 return DMUB_STATUS_INVALID; 418 419 dmub->fb_base = params->fb_base; 420 dmub->fb_offset = params->fb_offset; 421 dmub->psp_version = params->psp_version; 422 2. Condition dmub->hw_funcs.reset, taking true branch. 423 if (dmub->hw_funcs.reset) 424 dmub->hw_funcs.reset(dmub); 425 3. Condition inst_fb, taking true branch. 4. Condition data_fb, taking true branch. 426 if (inst_fb && data_fb) { 427 cw0.offset.quad_part = inst_fb->gpu_addr; 428 cw0.region.base = DMUB_CW0_BASE; 429 cw0.region.top = cw0.region.base + inst_fb->size - 1; 430 431 cw1.offset.quad_part = stack_fb->gpu_addr; 432 cw1.region.base = DMUB_CW1_BASE; 433 cw1.region.top = cw1.region.base + stack_fb->size - 1; 434 5. Condition params->load_inst_const, taking true branch. 6. Condition dmub->hw_funcs.backdoor_load, taking true branch. 435 if (params->load_inst_const && dmub->hw_funcs.backdoor_load) { 436 /** 437 * Read back all the instruction memory so we don't hang the 438 * DMCUB when backdoor loading if the write from x86 hasn't been 439 * flushed yet. This only occurs in backdoor loading. 440 */ 441 dmub_flush_buffer_mem(inst_fb); 442 dmub->hw_funcs.backdoor_load(dmub, &cw0, &cw1); 443 } 444 445 } 446 7. Condition inst_fb, taking true branch. 8. Condition data_fb, taking true branch. 9. Condition bios_fb, taking true branch. 10. Condition mail_fb, taking true branch. 11. Condition tracebuff_fb, taking false branch. 12. var_compare_op: Comparing tracebuff_fb to null implies that tracebuff_fb might be null. 447 if (inst_fb && data_fb && bios_fb && mail_fb && tracebuff_fb && 448 fw_state_fb && scratch_mem_fb) { 449 cw2.offset.quad_part = data_fb->gpu_addr; 450 cw2.region.base = DMUB_CW0_BASE + inst_fb->size; 451 cw2.region.top = cw2.region.base + data_fb->size; 452 453 cw3.offset.quad_part = bios_fb->gpu_addr; 454 cw3.region.base = DMUB_CW3_BASE; 455 cw3.region.top = cw3.region.base + bios_fb->size; 456 457 cw4.offset.quad_part = mail_fb->gpu_addr; 458 cw4.region.base = DMUB_CW4_BASE; 459 cw4.region.top = cw4.region.base + mail_fb->size; 460 461 /** 462 * Doubled the mailbox region to accomodate inbox and outbox. 463 * Note: Currently, currently total mailbox size is 16KB. It is split 464 * equally into 8KB between inbox and outbox. If this config is 465 * changed, then uncached base address configuration of outbox1 466 * has to be updated in funcs->setup_out_mailbox. 467 */ 468 inbox1.base = cw4.region.base; 469 inbox1.top = cw4.region.base + DMUB_RB_SIZE; 470 outbox1.base = inbox1.top; 471 outbox1.top = cw4.region.top; 472 473 cw5.offset.quad_part = tracebuff_fb->gpu_addr; 474 cw5.region.base = DMUB_CW5_BASE; 475 cw5.region.top = cw5.region.base + tracebuff_fb->size; 476 477 outbox0.base = DMUB_REGION5_BASE + TRACE_BUFFER_ENTRY_OFFSET; 478 outbox0.top = outbox0.base + sizeof(struct dmcub_trace_buf_entry) * PERF_TRACE_MAX_ENTRY; 479 480 481 cw6.offset.quad_part = fw_state_fb->gpu_addr; 482 cw6.region.base = DMUB_CW6_BASE; 483 cw6.region.top = cw6.region.base + fw_state_fb->size; 484 485 dmub->fw_state = fw_state_fb->cpu_addr; 486 487 dmub->scratch_mem_fb = *scratch_mem_fb; 488 489 if (dmub->hw_funcs.setup_windows) 490 dmub->hw_funcs.setup_windows(dmub, &cw2, &cw3, &cw4, 491 &cw5, &cw6); 492 493 if (dmub->hw_funcs.setup_outbox0) 494 dmub->hw_funcs.setup_outbox0(dmub, &outbox0); 495 496 if (dmub->hw_funcs.setup_mailbox) 497 dmub->hw_funcs.setup_mailbox(dmub, &inbox1); 498 if (dmub->hw_funcs.setup_out_mailbox) 499 dmub->hw_funcs.setup_out_mailbox(dmub, &outbox1); 500 } 501 13. Condition mail_fb, taking true branch. 502 if (mail_fb) { 503 dmub_memset(&rb_params, 0, sizeof(rb_params)); 504 rb_params.ctx = dmub; 505 rb_params.base_address = mail_fb->cpu_addr; 506 rb_params.capacity = DMUB_RB_SIZE; 507 508 dmub_rb_init(&dmub->inbox1_rb, &rb_params); 509 510 // Initialize outbox1 ring buffer 511 rb_params.ctx = dmub; 512 rb_params.base_address = (void *) ((uint64_t) (mail_fb->cpu_addr) + DMUB_RB_SIZE); 513 rb_params.capacity = DMUB_RB_SIZE; 514 dmub_rb_init(&dmub->outbox1_rb, &rb_params); 515 516 } 517 518 dmub_memset(&outbox0_rb_params, 0, sizeof(outbox0_rb_params)); 519 outbox0_rb_params.ctx = dmub; Dereference after null check (FORWARD_NULL) 14. var_deref_op: Dereferencing null pointer tracebuff_fb. 520 outbox0_rb_params.base_address = (void *)((uint64_t)(tracebuff_fb->cpu_addr) + TRACE_BUFFER_ENTRY_OFFSET); 521 outbox0_rb_params.capacity = sizeof(struct dmcub_trace_buf_entry) * PERF_TRACE_MAX_ENTRY; The check on line 447 implies that tracebuf_fb could possibly be null, however on line 520 tracebuf_fb is being dereferenced, so there is a possibly (perhaps unlikely) null pointer dereference issue here. Colin 522 dmub_rb_init(&dmub->outbox0_rb, &outbox0_rb_params); _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx