[Intel-gfx] [PATCH 3/4] drm/i915/bios: make sure to check vbt size

[Lucas De Marchi]

On Fri, Nov 08, 2019 at 12:08:52PM +0200, Jani Nikula wrote:
>On Thu, 07 Nov 2019, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> When we call intel_bios_is_valid_vbt(), size may not actually be the
>> size of the VBT, but rather the size of the blob the VBT is contained
>> in. For example, when mapping the PCI oprom, size will be the entire
>> oprom size. We don't want to read beyond what is reported to be the
>> VBT. So make sure we vbt->vbt_size makes sense and use that for
>> the latter checks.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index 1f83616cfc32..671bbce6ba5b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -1777,11 +1777,13 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
>>  	if (!vbt)
>>  		return false;
>>
>> -	if (sizeof(struct vbt_header) > size) {
>> +	if (sizeof(struct vbt_header) > size || vbt->vbt_size > size) {
>>  		DRM_DEBUG_DRIVER("VBT header incomplete\n");
>
>Nitpick #1, semantically you should check the VBT signature before you
>know ->vbt_size might make sense.
>
>Nitpick #2, the debug message becomes increasingly non-informative. But
>basically most messages in this function are less than stellar.

I can move this additional check after the signature check and then give
it a better error message. This is what I did in copy_vbt() anyway in
the next patch (but just for the pci rom).

thanks
Lucas De Marchi

>
>In any case, the goal is sane,
>
>Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>
>>  		return false;
>>  	}
>>
>> +	size = vbt->vbt_size;
>> +
>>  	if (memcmp(vbt->signature, "$VBT", 4)) {
>>  		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>>  		return false;
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center