Bug 108609

Created attachment 142298 [details] [review]
Patch to fix accessing mvdd_voltage_table.entries[] array out of bounds in vegam_smumgr.c

I believe I've discovered a small bug in the vegam_smumgr.c, specifically the following function:

static int vegam_populate_smc_mvdd_table(struct pp_hwmgr *hwmgr,
			SMU75_Discrete_DpmTable *table)
{
	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
	uint32_t count, level;

	if (SMU7_VOLTAGE_CONTROL_BY_GPIO == data->mvdd_control) {
		count = data->mvdd_voltage_table.count;
		if (count > SMU_MAX_SMIO_LEVELS)
			count = SMU_MAX_SMIO_LEVELS;
		for (level = 0; level < count; level++) {
			table->SmioTable2.Pattern[level].Voltage = PP_HOST_TO_SMC_US(
					data->mvdd_voltage_table.entries[count].value * VOLTAGE_SCALE);
			/* Index into DpmTable.Smio. Drive bits from Smio entry to get this voltage level.*/
			table->SmioTable2.Pattern[level].Smio =
				(uint8_t) level;
			table->Smio[level] |=
				data->mvdd_voltage_table.entries[level].smio_low;
		}
		table->SmioMask2 = data->mvdd_voltage_table.mask_low;

		table->MvddLevelCount = (uint32_t) PP_HOST_TO_SMC_UL(count);
	}

	return 0;
}

With the lines (within the for loop):

table->SmioTable2.Pattern[level].Voltage = PP_HOST_TO_SMC_US(
		data->mvdd_voltage_table.entries[count].value * VOLTAGE_SCALE);

If this code was executed it would try to access the mvdd_voltage_table.entries[] array out of bounds, because count > than the max value for level.

I believe:

data->mvdd_voltage_table.entries[count].value

should actually be:

data->mvdd_voltage_table.entries[level].value

You can see in a similar function within vegam_smumgr.c, this bug is *not* present:

static int vegam_populate_smc_vddci_table(struct pp_hwmgr *hwmgr,
					struct SMU75_Discrete_DpmTable *table)
{
	uint32_t count, level;
	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);

	count = data->vddci_voltage_table.count;

	if (SMU7_VOLTAGE_CONTROL_BY_GPIO == data->vddci_control) {
		if (count > SMU_MAX_SMIO_LEVELS)
			count = SMU_MAX_SMIO_LEVELS;
		for (level = 0; level < count; ++level) {
			table->SmioTable1.Pattern[level].Voltage = PP_HOST_TO_SMC_US(
					data->vddci_voltage_table.entries[level].value * VOLTAGE_SCALE);
			table->SmioTable1.Pattern[level].Smio = (uint8_t) level;

			table->Smio[level] |= data->vddci_voltage_table.entries[level].smio_low;
		}
	}

	table->SmioMask1 = data->vddci_voltage_table.mask_low;

	return 0;
}

I've attached a patch for kernel 4.19, admittedly the change is trivial but I figured I would try to do things the right way :)

Thanks!
Rob