lkml.org 
[lkml]   [2020]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [Intel-gfx][PATCH v2] drm/i915/gvt: Prevent NULL pointer dereference in intel_vgpu_reg_rw_edid()
Perfect!
I can confirm it resolves the issue as well, thank you very much.

Have a great day,
Alejandro Sior.


On Fri, 11 Sep 2020 at 07:58, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
>
> On 2020.09.08 20:11:21 +0200, Alejandro Sior wrote:
> > In the function intel_vgpu_reg_rw_edid of kvmgt.c, pos can be equal
> > to NULL for GPUs that do not properly support EDID. In those cases, when
> > pos gets passed to the handle_edid functions, it gets added a short offset
> > then it's dereferenced in memcpy's, leading to NULL pointer
> > dereference kernel oops.
> >
> > More concretely, that kernel oops renders some Broadwell GPUs users
> > unable to set up virtual machines with virtual GPU passthrough (virtual
> > machines hang indefinitely when trying to make use of the virtual GPU),
> > and make them unable to remove the virtual GPUs once the kernel oops has
> > happened (it hangs indefinitely, and notably too when the kernel tries to
> > shutdown). The issues that this causes and steps to reproduce are
> > discussed in more details in this github issue post:
> > https://github.com/intel/gvt-linux/issues/170#issuecomment-685806160
> >
> > Check if pos is equal to NULL, and if it is, set ret to a negative
> > value, making the module simply indicate that the access to EDID region
> > has failed, without any fatal repercussion.
> >
> > Signed-off-by: Alejandro Sior <aho@sior.be>
> >
> > ---
> > Changes in v2:
> > - removed middle name of author to comply with git name
> > - rephrased the patch description with imperative phrasing
> > - removed useless paragraph
> > - made a paragraph more concise
> > - fixed typos
> > - made individual lines shorter than 75 chars
> >
> > drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > index ad8a9df49f29..49163363ba4a 100644
> > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> > @@ -557,7 +557,9 @@ static size_t intel_vgpu_reg_rw_edid(struct intel_vgpu *vgpu, char *buf,
> > (struct vfio_edid_region *)kvmgt_vdev(vgpu)->region[i].data;
> > loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> >
> > - if (pos < region->vfio_edid_regs.edid_offset) {
> > + if (pos == NULL) {
> > + ret = -EINVAL;
> > + } else if (pos < region->vfio_edid_regs.edid_offset) {
> > ret = handle_edid_regs(vgpu, region, buf, count, pos, iswrite);
> > } else {
> > pos -= EDID_BLOB_OFFSET;
>
> Thanks for reporting this! Sorry that we failed to do sanity validation on older
> platform when enabling vfio edid function for all platform.
>
> Could you try below one to see if it fixes your problem? Instead of refusing
> possible read of edid, this fixes port number for BDW.
>
> From d5d9304b6bfdc31356fd2feb1ddbbf28073fe3d4 Mon Sep 17 00:00:00 2001
> From: Zhenyu Wang <zhenyuw@linux.intel.com>
> Date: Fri, 11 Sep 2020 13:50:20 +0800
> Subject: [PATCH] drm/i915/gvt: Fix port number for BDW on EDID region setup
>
> Current BDW virtual display port is initialized as PORT_B, so need
> to use same port for VFIO EDID region, otherwise invalid EDID blob
> pointer is assigned.
>
> Fixes: 0178f4ce3c3b ("drm/i915/gvt: Enable vfio edid for all GVT supported platform")
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gvt/vgpu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 8fa9b31a2484..f6d7e33c7099 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -368,6 +368,7 @@ void intel_gvt_destroy_idle_vgpu(struct intel_vgpu *vgpu)
> static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
> struct intel_vgpu_creation_params *param)
> {
> + struct drm_i915_private *dev_priv = gvt->gt->i915;
> struct intel_vgpu *vgpu;
> int ret;
>
> @@ -436,7 +437,10 @@ static struct intel_vgpu *__intel_gvt_create_vgpu(struct intel_gvt *gvt,
> if (ret)
> goto out_clean_sched_policy;
>
> - ret = intel_gvt_hypervisor_set_edid(vgpu, PORT_D);
> + if (IS_BROADWELL(dev_priv))
> + ret = intel_gvt_hypervisor_set_edid(vgpu, PORT_B);
> + else
> + ret = intel_gvt_hypervisor_set_edid(vgpu, PORT_D);
> if (ret)
> goto out_clean_sched_policy;
>
> --
> 2.28.0
>
>
>
> --
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

\
 
 \ /
  Last update: 2020-09-12 09:03    [W:0.033 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site