[Bug 108609] vegam_smumgr.c: accessing mvdd_voltage_table.entries[] array out of bounds in function vegam_populate_smc_mvdd_table

* [Bug 108609] vegam_smumgr.c: accessing mvdd_voltage_table.entries[] array out of bounds in function vegam_populate_smc_mvdd_table
@ 2018-10-31  6:55 bugzilla-daemon
  2018-10-31  8:47 ` bugzilla-daemon
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: bugzilla-daemon @ 2018-10-31  6:55 UTC (permalink / raw)
  To: dri-devel

[-- Attachment #1.1: Type: text/plain, Size: 4002 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=108609

            Bug ID: 108609
           Summary: vegam_smumgr.c: accessing mvdd_voltage_table.entries[]
                    array out of bounds in function
                    vegam_populate_smc_mvdd_table
           Product: DRI
           Version: unspecified
          Hardware: x86-64 (AMD64)
                OS: Linux (All)
            Status: NEW
          Severity: normal
          Priority: medium
         Component: DRM/AMDgpu
          Assignee: dri-devel@lists.freedesktop.org
          Reporter: rstrube@gmail.com

Created attachment 142298
  --> https://bugs.freedesktop.org/attachment.cgi?id=142298&action=edit
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

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 5770 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread