| Message ID | 20181120202439.13017-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/gvt: Avoid use-after-free iterating the gtt list | expand |
On 2018.11.20 20:24:38 +0000, Chris Wilson wrote: > Found by smatch: > > drivers/gpu/drm/i915/gvt/gtt.c:2452 intel_vgpu_destroy_ggtt_mm() error: dereferencing freed memory 'pos' > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com> > --- > drivers/gpu/drm/i915/gvt/gtt.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > index 58e166effa45..c7103dd2d8d5 100644 > --- a/drivers/gpu/drm/i915/gvt/gtt.c > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > @@ -2447,10 +2447,11 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu) > > static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) > { > - struct intel_gvt_partial_pte *pos; > + struct intel_gvt_partial_pte *pos, *next; > > - list_for_each_entry(pos, > - &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) { > + list_for_each_entry_safe(pos, next, > + &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, > + list) { > gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n", > pos->offset, pos->data); > kfree(pos); Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com> Thanks! I should really run check against each one when apply..
Sorry I missed it. Thanks for the correction! Regards, Henry > -----Original Message----- > From: Zhenyu Wang [mailto:zhenyuw@linux.intel.com] > Sent: Wednesday, November 21, 2018 10:29 AM > To: Chris Wilson <chris@chris-wilson.co.uk> > Cc: intel-gfx@lists.freedesktop.org; Zhenyu Wang > <zhenyuw@linux.intel.com>; Yuan, Hang <hang.yuan@intel.com> > Subject: Re: [PATCH 1/2] drm/i915/gvt: Avoid use-after-free iterating the gtt > list > > On 2018.11.20 20:24:38 +0000, Chris Wilson wrote: > > Found by smatch: > > > > drivers/gpu/drm/i915/gvt/gtt.c:2452 intel_vgpu_destroy_ggtt_mm() error: > dereferencing freed memory 'pos' > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com> > > --- > > drivers/gpu/drm/i915/gvt/gtt.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c > > b/drivers/gpu/drm/i915/gvt/gtt.c index 58e166effa45..c7103dd2d8d5 > > 100644 > > --- a/drivers/gpu/drm/i915/gvt/gtt.c > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > > @@ -2447,10 +2447,11 @@ static void > > intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu) > > > > static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) { > > - struct intel_gvt_partial_pte *pos; > > + struct intel_gvt_partial_pte *pos, *next; > > > > - list_for_each_entry(pos, > > - &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) { > > + list_for_each_entry_safe(pos, next, > > + &vgpu->gtt.ggtt_mm- > >ggtt_mm.partial_pte_list, > > + list) { > > gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n", > > pos->offset, pos->data); > > kfree(pos); > > Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com> > > Thanks! I should really run check against each one when apply.. > > -- > Open Source Technology Center, Intel ltd. > > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Quoting Zhenyu Wang (2018-11-21 02:29:21) > On 2018.11.20 20:24:38 +0000, Chris Wilson wrote: > > Found by smatch: > > > > drivers/gpu/drm/i915/gvt/gtt.c:2452 intel_vgpu_destroy_ggtt_mm() error: dereferencing freed memory 'pos' > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com> > > --- > > drivers/gpu/drm/i915/gvt/gtt.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > > index 58e166effa45..c7103dd2d8d5 100644 > > --- a/drivers/gpu/drm/i915/gvt/gtt.c > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > > @@ -2447,10 +2447,11 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu) > > > > static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) > > { > > - struct intel_gvt_partial_pte *pos; > > + struct intel_gvt_partial_pte *pos, *next; > > > > - list_for_each_entry(pos, > > - &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) { > > + list_for_each_entry_safe(pos, next, > > + &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, > > + list) { > > gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n", > > pos->offset, pos->data); > > kfree(pos); > > Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com> I presume you will take it via the gvt tree? Saves a backmerge for you. -Chris
On 2018.11.21 09:20:19 +0000, Chris Wilson wrote: > Quoting Zhenyu Wang (2018-11-21 02:29:21) > > On 2018.11.20 20:24:38 +0000, Chris Wilson wrote: > > > Found by smatch: > > > > > > drivers/gpu/drm/i915/gvt/gtt.c:2452 intel_vgpu_destroy_ggtt_mm() error: dereferencing freed memory 'pos' > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/gvt/gtt.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c > > > index 58e166effa45..c7103dd2d8d5 100644 > > > --- a/drivers/gpu/drm/i915/gvt/gtt.c > > > +++ b/drivers/gpu/drm/i915/gvt/gtt.c > > > @@ -2447,10 +2447,11 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu) > > > > > > static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) > > > { > > > - struct intel_gvt_partial_pte *pos; > > > + struct intel_gvt_partial_pte *pos, *next; > > > > > > - list_for_each_entry(pos, > > > - &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) { > > > + list_for_each_entry_safe(pos, next, > > > + &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, > > > + list) { > > > gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n", > > > pos->offset, pos->data); > > > kfree(pos); > > > > Reviewed-by: Zhenyu Wang <zhenyuw@linux.intel.com> > > I presume you will take it via the gvt tree? Saves a backmerge for you. Sure, will take it. thanks
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 58e166effa45..c7103dd2d8d5 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -2447,10 +2447,11 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu) static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu) { - struct intel_gvt_partial_pte *pos; + struct intel_gvt_partial_pte *pos, *next; - list_for_each_entry(pos, - &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) { + list_for_each_entry_safe(pos, next, + &vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, + list) { gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n", pos->offset, pos->data); kfree(pos);
Found by smatch: drivers/gpu/drm/i915/gvt/gtt.c:2452 intel_vgpu_destroy_ggtt_mm() error: dereferencing freed memory 'pos' Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Zhenyu Wang <zhenyuw@linux.intel.com> --- drivers/gpu/drm/i915/gvt/gtt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)