| Message ID | 20220803230325.137593-2-stuart.summers@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915: Fix NPD in PMU during driver teardown | expand |
On 04/08/2022 00:03, Stuart Summers wrote: > There can be a race in the PMU process teardown vs the > time when the driver is unbound in which the user attempts > to stop the PMU process, but the actual data structure > in the kernel is no longer available. Avoid this use-after-free > by skipping the PMU disable in i915_pmu_event_stop() when > the PMU has already been closed/unregistered by the driver. > > Fixes: b00bccb3f0bb ("drm/i915/pmu: Handle PCI unbind") > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > --- > drivers/gpu/drm/i915/i915_pmu.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c > index 958b37123bf12..0d02f338118e4 100644 > --- a/drivers/gpu/drm/i915/i915_pmu.c > +++ b/drivers/gpu/drm/i915/i915_pmu.c > @@ -760,9 +760,17 @@ static void i915_pmu_event_start(struct perf_event *event, int flags) > > static void i915_pmu_event_stop(struct perf_event *event, int flags) > { > + struct drm_i915_private *i915 = > + container_of(event->pmu, typeof(*i915), pmu.base); > + struct i915_pmu *pmu = &i915->pmu; > + > + if (pmu->closed) > + goto out; > + > if (flags & PERF_EF_UPDATE) > i915_pmu_event_read(event); > i915_pmu_disable(event); > +out: > event->hw.state = PERF_HES_STOPPED; > } > LGTM, although I am not sure who feels comfortable to r-b since we all kind of suggested the same fix. :) FWIW: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On Thu, 2022-08-04 at 09:46 +0100, Tvrtko Ursulin wrote: > On 04/08/2022 00:03, Stuart Summers wrote: > > There can be a race in the PMU process teardown vs the > > time when the driver is unbound in which the user attempts > > to stop the PMU process, but the actual data structure > > in the kernel is no longer available. Avoid this use-after-free > > by skipping the PMU disable in i915_pmu_event_stop() when > > the PMU has already been closed/unregistered by the driver. > > > > Fixes: b00bccb3f0bb ("drm/i915/pmu: Handle PCI unbind") > > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > > Signed-off-by: Stuart Summers <stuart.summers@intel.com> > > --- > > drivers/gpu/drm/i915/i915_pmu.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_pmu.c > > b/drivers/gpu/drm/i915/i915_pmu.c > > index 958b37123bf12..0d02f338118e4 100644 > > --- a/drivers/gpu/drm/i915/i915_pmu.c > > +++ b/drivers/gpu/drm/i915/i915_pmu.c > > @@ -760,9 +760,17 @@ static void i915_pmu_event_start(struct > > perf_event *event, int flags) > > > > static void i915_pmu_event_stop(struct perf_event *event, int > > flags) > > { > > + struct drm_i915_private *i915 = > > + container_of(event->pmu, typeof(*i915), pmu.base); > > + struct i915_pmu *pmu = &i915->pmu; > > + > > + if (pmu->closed) > > + goto out; > > + > > if (flags & PERF_EF_UPDATE) > > i915_pmu_event_read(event); > > i915_pmu_disable(event); > > +out: > > event->hw.state = PERF_HES_STOPPED; > > } > > > > LGTM, although I am not sure who feels comfortable to r-b since we > all > kind of suggested the same fix. :) > > FWIW: > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Thanks Tvrtko! I'll track down another reviewer here as well to close that out before merging. Thanks, Stuart > > Regards, > > Tvrtko
On Wed, Aug 03, 2022 at 11:03:25PM +0000, Stuart Summers wrote: >There can be a race in the PMU process teardown vs the >time when the driver is unbound in which the user attempts >to stop the PMU process, but the actual data structure >in the kernel is no longer available. Avoid this use-after-free >by skipping the PMU disable in i915_pmu_event_stop() when >the PMU has already been closed/unregistered by the driver. > >Fixes: b00bccb3f0bb ("drm/i915/pmu: Handle PCI unbind") >Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> >Signed-off-by: Stuart Summers <stuart.summers@intel.com> >--- > drivers/gpu/drm/i915/i915_pmu.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > >diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c >index 958b37123bf12..0d02f338118e4 100644 >--- a/drivers/gpu/drm/i915/i915_pmu.c >+++ b/drivers/gpu/drm/i915/i915_pmu.c >@@ -760,9 +760,17 @@ static void i915_pmu_event_start(struct perf_event *event, int flags) > > static void i915_pmu_event_stop(struct perf_event *event, int flags) > { >+ struct drm_i915_private *i915 = >+ container_of(event->pmu, typeof(*i915), pmu.base); >+ struct i915_pmu *pmu = &i915->pmu; >+ >+ if (pmu->closed) >+ goto out; >+ > if (flags & PERF_EF_UPDATE) > i915_pmu_event_read(event); > i915_pmu_disable(event); >+out: > event->hw.state = PERF_HES_STOPPED; > } lgtm Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Thanks, Umesh > >-- >2.25.1 >
diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c index 958b37123bf12..0d02f338118e4 100644 --- a/drivers/gpu/drm/i915/i915_pmu.c +++ b/drivers/gpu/drm/i915/i915_pmu.c @@ -760,9 +760,17 @@ static void i915_pmu_event_start(struct perf_event *event, int flags) static void i915_pmu_event_stop(struct perf_event *event, int flags) { + struct drm_i915_private *i915 = + container_of(event->pmu, typeof(*i915), pmu.base); + struct i915_pmu *pmu = &i915->pmu; + + if (pmu->closed) + goto out; + if (flags & PERF_EF_UPDATE) i915_pmu_event_read(event); i915_pmu_disable(event); +out: event->hw.state = PERF_HES_STOPPED; }
There can be a race in the PMU process teardown vs the time when the driver is unbound in which the user attempts to stop the PMU process, but the actual data structure in the kernel is no longer available. Avoid this use-after-free by skipping the PMU disable in i915_pmu_event_stop() when the PMU has already been closed/unregistered by the driver. Fixes: b00bccb3f0bb ("drm/i915/pmu: Handle PCI unbind") Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Signed-off-by: Stuart Summers <stuart.summers@intel.com> --- drivers/gpu/drm/i915/i915_pmu.c | 8 ++++++++ 1 file changed, 8 insertions(+)