Re: [PATCH 06/14] drm/i915: Validate VBT header before trusting it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/25/2014 1:32 PM, Daniel Vetter wrote:
On Thu, Apr 24, 2014 at 09:22:23PM +0530, Kumar, Shobhit wrote:
On 4/19/2014 2:34 AM, Rodrigo Vivi wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

Be we read and chase pointers from the VBT, it is prudent to make sure
that those accesses are wholly contained within the MMIO region, or else
we may cause a kernel panic during boot.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_bios.c | 68 ++++++++++++++++++++++++++++-----------
  1 file changed, 50 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index fba9efd..fc9e806 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1099,6 +1099,46 @@ static const struct dmi_system_id intel_no_opregion_vbt[] = {
  	{ }
  };

+static struct bdb_header *validate_vbt(char *base, size_t size,
+				       struct vbt_header *vbt,
+				       const char *source)
+{
+	size_t offset;
+	struct bdb_header *bdb;
+
+	if (vbt == NULL) {
+		DRM_DEBUG_DRIVER("VBT signature missing\n");
+		return NULL;
+	}
+
+	offset = (char *)vbt - base;
+	if (offset + sizeof(struct vbt_header) > size) {
+		DRM_DEBUG_DRIVER("VBT header incomplete\n");
+		return NULL;
+	}
+
+	if (memcmp(vbt->signature, "$VBT", 4)) {
+		DRM_DEBUG_DRIVER("VBT invalid signature\n");
+		return NULL;
+	}
+
+	offset += vbt->bdb_offset;
+	if (offset + sizeof(struct bdb_header) > size) {
+		DRM_DEBUG_DRIVER("BDB header incomplete\n");
+		return NULL;
+	}
+
+	bdb = (struct bdb_header *)(base + offset);
+	if (offset + bdb->bdb_size > size) {
+		DRM_DEBUG_DRIVER("BDB incomplete\n");
+		return NULL;
+	}

I know that BDB version check is really not enough and VBT should be forward
compatible, but it would be good to have a version check in driver for the
current BDB version the parser supports as well. Strictly speaking if we
put this check we should ideally reject any newer versions, but putting an
error log indicating mismatch might be a  good idea for debug.

Sounds more like an additional patch on top of this one here, so still r-b
from your side for these changes here?
-Daniel


Hmm, I felt it is something which is missed as part of this patch while validating VBT. But yes if you feel that it is okay to be a separate patch then for these changes -

Reviewed-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>

I will then push a new patch on version check.

Regards
Shobhit
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux