Hello Alex Deucher,
The patch a2e73f56fa62: "drm/amdgpu: Add support for CIK parts" from
Apr 20, 2015, leads to the following static checker warning:
drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4509 ci_set_mc_special_registers()
error: buffer overflow 'table->mc_reg_address' 16 <= 16
drivers/gpu/drm/amd/amdgpu/ci_dpm.c
4491 j++;
4492 if (j >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
4493 return -EINVAL;
4494
4495 temp_reg = RREG32(mmMC_PMG_CMD_MRS);
4496 table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
4497 table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;
4498 for (k = 0; k < table->num_entries; k++) {
4499 table->mc_reg_table_entry[k].mc_data[j] =
4500 (temp_reg & 0xffff0000) | (table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
4501 if (adev->mc.vram_type != AMDGPU_VRAM_TYPE_GDDR5)
4502 table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
4503 }
4504 j++;
4505 if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
This should >= instead of >.
4506 return -EINVAL;
4507
4508 if (adev->mc.vram_type != AMDGPU_VRAM_TYPE_GDDR5) {
4509 table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
4510 table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
4511 for (k = 0; k < table->num_entries; k++) {
4512 table->mc_reg_table_entry[k].mc_data[j] =
4513 (table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
4514 }
4515 j++;
4516 if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
4517 return -EINVAL;
But my question was about the other checks, should they be changed as
well?
4518 }
4519 break;
4520 case mmMC_SEQ_RESERVE_M:
4521 temp_reg = RREG32(mmMC_PMG_CMD_MRS1);
4522 table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS1;
4523 table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS1_LP;
4524 for (k = 0; k < table->num_entries; k++) {
4525 table->mc_reg_table_entry[k].mc_data[j] =
4526 (temp_reg & 0xffff0000) | (table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
4527 }
4528 j++;
4529 if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
4530 return -EINVAL;
4531 break;
4532 default:
4533 break;
4534 }
4535
4536 }
4537
4538 table->last = j;
We use the last j here. It's not clear to me if ->last has to be within
the array. I would have generally assumed it would be except for the
check below.
4539
4540 return 0;
4541 }
4542
[ snip ]
4670 static int ci_register_patching_mc_seq(struct amdgpu_device *adev,
4671 struct ci_mc_reg_table *table)
4672 {
4673 u8 i, k;
4674 u32 tmp;
4675 bool patch;
4676
4677 tmp = RREG32(mmMC_SEQ_MISC0);
4678 patch = ((tmp & 0x0000f00) == 0x300) ? true : false;
4679
4680 if (patch &&
4681 ((adev->pdev->device == 0x67B0) ||
4682 (adev->pdev->device == 0x67B1))) {
4683 for (i = 0; i < table->last; i++) {
4684 if (table->last >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
Here, we assume ->last has to be inside the array.
4685 return -EINVAL;
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
