Re: [PATCH v2] i915/perf: Fix NULL deref bugs with drm_dbg() calls

[Tvrtko Ursulin]

On 27/10/2023 18:38, Tvrtko Ursulin wrote:
> On 27/10/2023 18:28, Harshit Mogalapalli wrote:
> > When i915 perf interface is not available dereferencing it will lead to
> > NULL dereferences.
> >
> > As returning -ENOTSUPP is pretty clear return when perf interface is not
> > available.
> >
> > Fixes: 2fec539112e8 ("i915/perf: Replace DRM_DEBUG with driver 
> > specific drm_dbg call")
> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@xxxxxxxxxx>
> > ---
> > v1 --> v2: Remove the debug calls as they don't add much value and
> > -ENOTSUPP is a good enough return value.
> > ---
> >   drivers/gpu/drm/i915/i915_perf.c | 15 +++------------
> >   1 file changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c 
> > b/drivers/gpu/drm/i915/i915_perf.c
> > index 2f3ecd7d4804..7b1c8de2f9cb 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -4227,11 +4227,8 @@ int i915_perf_open_ioctl(struct drm_device 
> > *dev, void *data,
> >       u32 known_open_flags;
> >       int ret;
> > -    if (!perf->i915) {
> > -        drm_dbg(&perf->i915->drm,
> > -            "i915 perf interface not available for this system\n");
> > +    if (!perf->i915)
> >           return -ENOTSUPP;
> > -    }
> >       known_open_flags = I915_PERF_FLAG_FD_CLOEXEC |
> >                  I915_PERF_FLAG_FD_NONBLOCK |
> > @@ -4607,11 +4604,8 @@ int i915_perf_add_config_ioctl(struct 
> > drm_device *dev, void *data,
> >       struct i915_oa_reg *regs;
> >       int err, id;
> > -    if (!perf->i915) {
> > -        drm_dbg(&perf->i915->drm,
> > -            "i915 perf interface not available for this system\n");
> > +    if (!perf->i915)
> >           return -ENOTSUPP;
> > -    }
> >       if (!perf->metrics_kobj) {
> >           drm_dbg(&perf->i915->drm,
> > @@ -4773,11 +4767,8 @@ int i915_perf_remove_config_ioctl(struct 
> > drm_device *dev, void *data,
> >       struct i915_oa_config *oa_config;
> >       int ret;
> > -    if (!perf->i915) {
> > -        drm_dbg(&perf->i915->drm,
> > -            "i915 perf interface not available for this system\n");
> > +    if (!perf->i915)
> >           return -ENOTSUPP;
> > -    }
> >       if (i915_perf_stream_paranoid && !perfmon_capable()) {
> >           drm_dbg(&perf->i915->drm,
>
> Thanks for re-spinning it so quickly! LGTM.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Now merged to drm-intel-gt-next. Thanks again!

Regards,

Tvrtko