Re: [PATCH AUTOSEL 6.18] drm/amdgpu: validate RAS EEPROM tbl_size before record count

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Unvalidated `first_rec_offset` from EEPROM leads to an unsigned underflow in `RAS_OFFSET_TO_INDEX`, causing a massive heap buffer overflow during `amdgpu_ras_eeprom_read`.
--

commit 887663ae59b09548ad94227f6b760b666b20f6c3
Author: Candice Li <candice.li@amd.com>

drm/amdgpu: validate RAS EEPROM tbl_size before record count

Corrupt EEPROM data can set tbl_size below the table header size.
Guard the RAS_NUM_RECS macros against undersized tbl_size and reset
the table during init when tbl_size is below the minimum for the table
version instead of trusting the header.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> index 3eb3fb55ccb05..ef4389deb24ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c

[ ... ]

> @@ -1415,11 +1418,24 @@ int amdgpu_ras_eeprom_init(struct amdgpu_ras_eeprom_control *control)
>  	switch (hdr->version) {
>  	case RAS_TABLE_VER_V2_1:
>  	case RAS_TABLE_VER_V3:
> +		if (hdr->tbl_size < RAS_TABLE_HEADER_SIZE + RAS_TABLE_V2_1_INFO_SIZE) {
> +			dev_err(adev->dev,
> +				"RAS header invalid, tbl_size %u smaller than minimum %u, resetting table\n",
> +				hdr->tbl_size,
> +				RAS_TABLE_HEADER_SIZE + RAS_TABLE_V2_1_INFO_SIZE);
> +			return amdgpu_ras_eeprom_reset_table(control);
> +		}
>  		control->ras_num_recs = RAS_NUM_RECS_V2_1(hdr);
>  		control->ras_record_offset = RAS_RECORD_START_V2_1;
>  		control->ras_max_record_count = RAS_MAX_RECORD_COUNT_V2_1;
>  		break;
>  	case RAS_TABLE_VER_V1:
> +		if (hdr->tbl_size < RAS_TABLE_HEADER_SIZE) {
> +			dev_err(adev->dev,
> +				"RAS header invalid, tbl_size %u smaller than minimum %u, resetting table\n",
> +				hdr->tbl_size, RAS_TABLE_HEADER_SIZE);
> +			return amdgpu_ras_eeprom_reset_table(control);
> +		}
>  		control->ras_num_recs = RAS_NUM_RECS(hdr);
>  		control->ras_record_offset = RAS_RECORD_START;
>  		control->ras_max_record_count = RAS_MAX_RECORD_COUNT;

[Severity: Critical]
This is a pre-existing issue, but since this patch handles validation of
corrupt EEPROM data, I wanted to point out another potential issue in this
function.

A bit further down in amdgpu_ras_eeprom_init(), first_rec_offset is used
without bounds validation:

drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c:amdgpu_ras_eeprom_init() {
    ...
	control->ras_fri = RAS_OFFSET_TO_INDEX(control, hdr->first_rec_offset);
    ...
}

If a corrupted or maliciously programmed EEPROM provides a first_rec_offset
that is smaller than control->ras_record_offset (e.g., 0), the unsigned
subtraction inside the RAS_OFFSET_TO_INDEX macro could underflow, assigning
a massive value to control->ras_fri.

Later, when amdgpu_ras_eeprom_read() attempts to read the records, this
underflow could propagate and cause a large heap buffer overflow:

drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c:amdgpu_ras_eeprom_read() {
	...
	} else {
		g0 = control->ras_max_record_count - control->ras_fri;
		g1 += 1;
	}
	...
	res = __amdgpu_ras_eeprom_read(control, buf, control->ras_fri, g0);
	...
}

The variable g0 would again underflow to a massive value, which is then
passed as the num parameter to __amdgpu_ras_eeprom_read():

drivers/gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c:__amdgpu_ras_eeprom_read() {
	...
	buf_size = num * RAS_TABLE_RECORD_SIZE;
	res = amdgpu_eeprom_read(adev->pm.ras_eeprom_i2c_bus,
				 control->i2c_address +
				 RAS_INDEX_TO_OFFSET(control, fri),
				 buf, buf_size);
	...
}

The massive calculated buf_size is passed to amdgpu_eeprom_read(), which
could continuously read from the I2C bus directly into the small,
correctly-sized heap buffer, leading to memory corruption.

Could hdr->first_rec_offset be validated against control->ras_record_offset
during amdgpu_ras_eeprom_init() to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-69-sashal@kernel.org?part=1