[PATCH 2/3] drm/i915/perf: Whitelist OA report trigger registers

[Chris Wilson]

Quoting Umesh Nerlige Ramappa (2020-09-29 00:18:32)
> OA reports can be triggered into the OA buffer by writing into the
> OAREPORTTRIG registers. Whitelist the registers to allow non-privileged
> user to trigger reports.
> 
> Whitelist registers only if perf_stream_paranoid is set to 0. In
> i915_perf_open_ioctl, this setting is checked and the whitelist is
> enabled accordingly. On closing the perf fd, the whitelist is removed.
> 
> This ensures that the access to the whitelist is gated by
> perf_stream_paranoid.
> 
> v2:
> - Move related change to this patch (Lionel)
> - Bump up perf revision (Lionel)
> 
> v3: Pardon whitelisted registers for selftest (Umesh)
> v4: Document supported gens for the feature (Lionel)
> v5: Whitelist registers only if perf_stream_paranoid is set to 0 (Jon)
> v6: Move oa whitelist array to i915_perf (Chris)
> v7: Fix OA writing beyond the wal->list memory (CI)
> v8: Protect write to engine whitelist registers
> v9: (Umesh)
> - Use uncore->lock to protect write to forcepriv regs
> - In case wal->count falls to zero on _wa_remove, make sure you still
>   clear the registers. Remove wal->count check when applying whitelist.
> 
> Signed-off-by: Piotr Maciejewski <piotr.maciejewski at intel.com>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa at intel.com>
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   7 +-
>  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 127 ++++++++++++++++--
>  drivers/gpu/drm/i915/gt/intel_workarounds.h   |   9 +-
>  .../gpu/drm/i915/gt/intel_workarounds_types.h |   5 +
>  drivers/gpu/drm/i915/i915_perf.c              |  76 ++++++++++-
>  drivers/gpu/drm/i915/i915_perf_types.h        |   5 +
>  6 files changed, 211 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 5bfb5f7ed02c..00f62c8f1a7e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -951,8 +951,13 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
>   */
>  int intel_engine_resume(struct intel_engine_cs *engine)
>  {
> +       unsigned long flags;
> +
>         intel_engine_apply_workarounds(engine);
> -       intel_engine_apply_whitelist(engine);
> +
> +       spin_lock_irqsave(&engine->uncore->lock, flags);
> +       intel_engine_apply_whitelist(engine, false);
> +       spin_unlock_irqrestore(&engine->uncore->lock, flags);
>  
>         return engine->resume(engine);
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 39817c5a7058..982589be9046 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -91,12 +91,23 @@ static void wa_init_start(struct i915_wa_list *wal, const char *name, const char
>  
>  #define WA_LIST_CHUNK (1 << 4)
>  
> +/*
> + * Some of the i915 code like perf OA tries to whitelist registers on demand.
> + * Such code adds to the wal->list, but that would not work because the list
> + * is compacted below by wa_init_finish. While _wa_add does have code to grow
> + * the list, it does not seem to take the compaction into consideration. Leave
> + * 8 entries free during the compaction until a better mechanism can be put in
> + * place.
> + */
> +#define WA_LIST_DYNAMIC_ENTRIES 8
> +
>  static void wa_init_finish(struct i915_wa_list *wal)
>  {
>         /* Trim unused entries. */
>         if (!IS_ALIGNED(wal->count, WA_LIST_CHUNK)) {
> +               size_t size = wal->count + WA_LIST_DYNAMIC_ENTRIES;
>                 struct i915_wa *list = kmemdup(wal->list,
> -                                              wal->count * sizeof(*list),
> +                                              size * sizeof(*list),

This is giving kasan a heart attack for the out-of-bounds read.
-Chris