Commit a6582701 authored by Zhipeng Lu's avatar Zhipeng Lu Committed by Alex Deucher

drm/amd/pm: fix a double-free in amdgpu_parse_extended_power_table

The amdgpu_free_extended_power_table is called in every error-handling
paths of amdgpu_parse_extended_power_table. However, after the following
call chain of returning:

amdgpu_parse_extended_power_table
  |-> kv_dpm_init / si_dpm_init
      (the only two caller of amdgpu_parse_extended_power_table)
        |-> kv_dpm_sw_init / si_dpm_sw_init
            (the only caller of kv_dpm_init / si_dpm_init, accordingly)
              |-> kv_dpm_fini / si_dpm_fini
                  (goto dpm_failed in xx_dpm_sw_init)
                    |-> amdgpu_free_extended_power_table

As above, the amdgpu_free_extended_power_table is called twice in this
returning chain and thus a double-free is triggered. Similarily, the
last kfree in amdgpu_parse_extended_power_table also cause a double free
with amdgpu_free_extended_power_table in kv_dpm_fini.

Fixes: 84176663 ("drm/amd/pm: create a new holder for those APIs used only by legacy ASICs(si/kv)")
Signed-off-by: default avatarZhipeng Lu <alexious@zju.edu.cn>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent c2709b2d
...@@ -272,10 +272,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -272,10 +272,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
le16_to_cpu(power_info->pplib4.usVddcDependencyOnSCLKOffset)); le16_to_cpu(power_info->pplib4.usVddcDependencyOnSCLKOffset));
ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddc_dependency_on_sclk, ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddc_dependency_on_sclk,
dep_table); dep_table);
if (ret) { if (ret)
amdgpu_free_extended_power_table(adev);
return ret; return ret;
}
} }
if (power_info->pplib4.usVddciDependencyOnMCLKOffset) { if (power_info->pplib4.usVddciDependencyOnMCLKOffset) {
dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *) dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *)
...@@ -283,10 +281,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -283,10 +281,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
le16_to_cpu(power_info->pplib4.usVddciDependencyOnMCLKOffset)); le16_to_cpu(power_info->pplib4.usVddciDependencyOnMCLKOffset));
ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddci_dependency_on_mclk, ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddci_dependency_on_mclk,
dep_table); dep_table);
if (ret) { if (ret)
amdgpu_free_extended_power_table(adev);
return ret; return ret;
}
} }
if (power_info->pplib4.usVddcDependencyOnMCLKOffset) { if (power_info->pplib4.usVddcDependencyOnMCLKOffset) {
dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *) dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *)
...@@ -294,10 +290,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -294,10 +290,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
le16_to_cpu(power_info->pplib4.usVddcDependencyOnMCLKOffset)); le16_to_cpu(power_info->pplib4.usVddcDependencyOnMCLKOffset));
ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddc_dependency_on_mclk, ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.vddc_dependency_on_mclk,
dep_table); dep_table);
if (ret) { if (ret)
amdgpu_free_extended_power_table(adev);
return ret; return ret;
}
} }
if (power_info->pplib4.usMvddDependencyOnMCLKOffset) { if (power_info->pplib4.usMvddDependencyOnMCLKOffset) {
dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *) dep_table = (ATOM_PPLIB_Clock_Voltage_Dependency_Table *)
...@@ -305,10 +299,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -305,10 +299,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
le16_to_cpu(power_info->pplib4.usMvddDependencyOnMCLKOffset)); le16_to_cpu(power_info->pplib4.usMvddDependencyOnMCLKOffset));
ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.mvdd_dependency_on_mclk, ret = amdgpu_parse_clk_voltage_dep_table(&adev->pm.dpm.dyn_state.mvdd_dependency_on_mclk,
dep_table); dep_table);
if (ret) { if (ret)
amdgpu_free_extended_power_table(adev);
return ret; return ret;
}
} }
if (power_info->pplib4.usMaxClockVoltageOnDCOffset) { if (power_info->pplib4.usMaxClockVoltageOnDCOffset) {
ATOM_PPLIB_Clock_Voltage_Limit_Table *clk_v = ATOM_PPLIB_Clock_Voltage_Limit_Table *clk_v =
...@@ -339,10 +331,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -339,10 +331,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
kcalloc(psl->ucNumEntries, kcalloc(psl->ucNumEntries,
sizeof(struct amdgpu_phase_shedding_limits_entry), sizeof(struct amdgpu_phase_shedding_limits_entry),
GFP_KERNEL); GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.phase_shedding_limits_table.entries) { if (!adev->pm.dpm.dyn_state.phase_shedding_limits_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
entry = &psl->entries[0]; entry = &psl->entries[0];
for (i = 0; i < psl->ucNumEntries; i++) { for (i = 0; i < psl->ucNumEntries; i++) {
...@@ -383,10 +373,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -383,10 +373,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
ATOM_PPLIB_CAC_Leakage_Record *entry; ATOM_PPLIB_CAC_Leakage_Record *entry;
u32 size = cac_table->ucNumEntries * sizeof(struct amdgpu_cac_leakage_table); u32 size = cac_table->ucNumEntries * sizeof(struct amdgpu_cac_leakage_table);
adev->pm.dpm.dyn_state.cac_leakage_table.entries = kzalloc(size, GFP_KERNEL); adev->pm.dpm.dyn_state.cac_leakage_table.entries = kzalloc(size, GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.cac_leakage_table.entries) { if (!adev->pm.dpm.dyn_state.cac_leakage_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
entry = &cac_table->entries[0]; entry = &cac_table->entries[0];
for (i = 0; i < cac_table->ucNumEntries; i++) { for (i = 0; i < cac_table->ucNumEntries; i++) {
if (adev->pm.dpm.platform_caps & ATOM_PP_PLATFORM_CAP_EVV) { if (adev->pm.dpm.platform_caps & ATOM_PP_PLATFORM_CAP_EVV) {
...@@ -438,10 +426,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -438,10 +426,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
sizeof(struct amdgpu_vce_clock_voltage_dependency_entry); sizeof(struct amdgpu_vce_clock_voltage_dependency_entry);
adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.entries = adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.entries =
kzalloc(size, GFP_KERNEL); kzalloc(size, GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.entries) { if (!adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.count = adev->pm.dpm.dyn_state.vce_clock_voltage_dependency_table.count =
limits->numEntries; limits->numEntries;
entry = &limits->entries[0]; entry = &limits->entries[0];
...@@ -493,10 +479,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -493,10 +479,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
sizeof(struct amdgpu_uvd_clock_voltage_dependency_entry); sizeof(struct amdgpu_uvd_clock_voltage_dependency_entry);
adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.entries = adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.entries =
kzalloc(size, GFP_KERNEL); kzalloc(size, GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.entries) { if (!adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.count = adev->pm.dpm.dyn_state.uvd_clock_voltage_dependency_table.count =
limits->numEntries; limits->numEntries;
entry = &limits->entries[0]; entry = &limits->entries[0];
...@@ -525,10 +509,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -525,10 +509,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
sizeof(struct amdgpu_clock_voltage_dependency_entry); sizeof(struct amdgpu_clock_voltage_dependency_entry);
adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.entries = adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.entries =
kzalloc(size, GFP_KERNEL); kzalloc(size, GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.entries) { if (!adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.count = adev->pm.dpm.dyn_state.samu_clock_voltage_dependency_table.count =
limits->numEntries; limits->numEntries;
entry = &limits->entries[0]; entry = &limits->entries[0];
...@@ -548,10 +530,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -548,10 +530,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
le16_to_cpu(ext_hdr->usPPMTableOffset)); le16_to_cpu(ext_hdr->usPPMTableOffset));
adev->pm.dpm.dyn_state.ppm_table = adev->pm.dpm.dyn_state.ppm_table =
kzalloc(sizeof(struct amdgpu_ppm_table), GFP_KERNEL); kzalloc(sizeof(struct amdgpu_ppm_table), GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.ppm_table) { if (!adev->pm.dpm.dyn_state.ppm_table)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
adev->pm.dpm.dyn_state.ppm_table->ppm_design = ppm->ucPpmDesign; adev->pm.dpm.dyn_state.ppm_table->ppm_design = ppm->ucPpmDesign;
adev->pm.dpm.dyn_state.ppm_table->cpu_core_number = adev->pm.dpm.dyn_state.ppm_table->cpu_core_number =
le16_to_cpu(ppm->usCpuCoreNumber); le16_to_cpu(ppm->usCpuCoreNumber);
...@@ -583,10 +563,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -583,10 +563,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
sizeof(struct amdgpu_clock_voltage_dependency_entry); sizeof(struct amdgpu_clock_voltage_dependency_entry);
adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.entries = adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.entries =
kzalloc(size, GFP_KERNEL); kzalloc(size, GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.entries) { if (!adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.entries)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.count = adev->pm.dpm.dyn_state.acp_clock_voltage_dependency_table.count =
limits->numEntries; limits->numEntries;
entry = &limits->entries[0]; entry = &limits->entries[0];
...@@ -606,10 +584,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -606,10 +584,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
ATOM_PowerTune_Table *pt; ATOM_PowerTune_Table *pt;
adev->pm.dpm.dyn_state.cac_tdp_table = adev->pm.dpm.dyn_state.cac_tdp_table =
kzalloc(sizeof(struct amdgpu_cac_tdp_table), GFP_KERNEL); kzalloc(sizeof(struct amdgpu_cac_tdp_table), GFP_KERNEL);
if (!adev->pm.dpm.dyn_state.cac_tdp_table) { if (!adev->pm.dpm.dyn_state.cac_tdp_table)
amdgpu_free_extended_power_table(adev);
return -ENOMEM; return -ENOMEM;
}
if (rev > 0) { if (rev > 0) {
ATOM_PPLIB_POWERTUNE_Table_V1 *ppt = (ATOM_PPLIB_POWERTUNE_Table_V1 *) ATOM_PPLIB_POWERTUNE_Table_V1 *ppt = (ATOM_PPLIB_POWERTUNE_Table_V1 *)
(mode_info->atom_context->bios + data_offset + (mode_info->atom_context->bios + data_offset +
...@@ -645,10 +621,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev) ...@@ -645,10 +621,8 @@ int amdgpu_parse_extended_power_table(struct amdgpu_device *adev)
ret = amdgpu_parse_clk_voltage_dep_table( ret = amdgpu_parse_clk_voltage_dep_table(
&adev->pm.dpm.dyn_state.vddgfx_dependency_on_sclk, &adev->pm.dpm.dyn_state.vddgfx_dependency_on_sclk,
dep_table); dep_table);
if (ret) { if (ret)
kfree(adev->pm.dpm.dyn_state.vddgfx_dependency_on_sclk.entries);
return ret; return ret;
}
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment