[3/4] drm/i915/bios: make sure to check vbt size
diff mbox series

Message ID 20191108003602.33526-3-lucas.demarchi@intel.com
State New
Headers show
Series
  • [1/4] drm/i915/opregion: fix leaking fw on error path
Related show

Commit Message

Lucas De Marchi Nov. 8, 2019, 12:36 a.m. UTC
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@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jani Nikula Nov. 8, 2019, 10:08 a.m. UTC | #1
On Thu, 07 Nov 2019, Lucas De Marchi <lucas.demarchi@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@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.

In any case, the goal is sane,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  		return false;
>  	}
>  
> +	size = vbt->vbt_size;
> +
>  	if (memcmp(vbt->signature, "$VBT", 4)) {
>  		DRM_DEBUG_DRIVER("VBT invalid signature\n");
>  		return false;

Patch
diff mbox series

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");
 		return false;
 	}
 
+	size = vbt->vbt_size;
+
 	if (memcmp(vbt->signature, "$VBT", 4)) {
 		DRM_DEBUG_DRIVER("VBT invalid signature\n");
 		return false;