On Fri, Apr 25, 2014 at 01:54:06PM +0530, Kumar, Shobhit wrote:
> 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 <ch...@chris-wilson.co.uk>
> >>>
> >>>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 <ch...@chris-wilson.co.uk>
> >>>Signed-off-by: Rodrigo Vivi <rodrigo.v...@gmail.com>
> >>>---
> >>>  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.ku...@intel.com>

Thanks, patch merged.

> I will then push a new patch on version check.

I've figured Chris could do it, but if you can take this over I don't
think Chris will complain ;-)

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to