[PATCH 1/4] drm/i915/display: Fix out-of-bounds access in pipe-related tracepoints
Matt Roper
matthew.d.roper at intel.com
Wed Sep 18 22:13:37 UTC 2024
On Thu, Aug 29, 2024 at 07:00:44PM -0300, Gustavo Sousa wrote:
> Some display trace events use array members to store frame and scanline
> counts for each pipe. However, those arrays are declared with 3 as the
> hardcoded size, which cause out-of-bounds access when the trace event is
> enabled on a platform that contains pipe D.
>
> For example, when looking at the last 10 intel_pipe_enable events after
> running IGT's testdisplay, we see the following on a MTL machine that
> has pipe D available:
>
> $ trace-cmd report -R -F intel_pipe_enable \
> > | tail \
> > | sed 's,\(frame=.*\) \(scanline=.*\),\n\t \1\n\t\2,'
> testdisplay-6715 [002] 17591.063491: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[83, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [003] 17591.264742: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[89, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [003] 17591.464541: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[8f, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [001] 17591.695827: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[95, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [000] 17591.915841: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[9a, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [000] 17592.127114: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[a0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [002] 17592.358351: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[a8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [002] 17592.580467: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[ae, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [000] 17592.950946: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[b8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-6715 [004] 17593.079597: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[bf, 01, 00, 00, 01, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[00, 00, 00, 00, 3a, 04, 00, 00, 00, 00, 00, 00] pipe=1
>
> Which shows zeros for pipe A's scanline counts. That happens because
> pipe D's frame counts are overwriting them.
>
> Let's fix that by making the arrays bring able to store info for all
> possible pipes.
>
> With the fix, we get the following:
>
> $ trace-cmd report -R -F intel_pipe_enable \
> > | tail \
> > | sed 's,\(frame=.*\) \(scanline=.*\),\n\t \1\n\t\2,'
> testdisplay-7040 [003] 18067.489565: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[8c, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[8e, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18067.699312: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[92, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18067.908868: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[98, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18068.122802: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[9d, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [003] 18068.331019: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[a2, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18068.529067: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[a8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [003] 18068.742033: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[ae, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18068.956229: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[b3, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[1f, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [002] 18069.295322: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[bb, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[7b, 08, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0
> testdisplay-7040 [010] 18069.423527: intel_pipe_enable: dev=0000:00:02.0
> frame=ARRAY[c2, 01, 00, 00, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00]
> scanline=ARRAY[d0, 05, 00, 00, 3a, 04, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=1
>
> Which makes more sense now.
>
> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
I guess nobody has really needed to use these tracepoints much for
debugging since TGL added the 4th pipe.
Matt
> ---
> drivers/gpu/drm/i915/display/intel_display_trace.h | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h
> index c734ef1fba3c..8a3185862089 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_trace.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h
> @@ -15,6 +15,7 @@
>
> #include "i915_drv.h"
> #include "intel_crtc.h"
> +#include "intel_display_limits.h"
> #include "intel_display_types.h"
> #include "intel_vblank.h"
>
> @@ -27,8 +28,8 @@ TRACE_EVENT(intel_pipe_enable,
>
> TP_STRUCT__entry(
> __string(dev, __dev_name_kms(crtc))
> - __array(u32, frame, 3)
> - __array(u32, scanline, 3)
> + __array(u32, frame, I915_MAX_PIPES)
> + __array(u32, scanline, I915_MAX_PIPES)
> __field(enum pipe, pipe)
> ),
> TP_fast_assign(
> @@ -55,8 +56,8 @@ TRACE_EVENT(intel_pipe_disable,
>
> TP_STRUCT__entry(
> __string(dev, __dev_name_kms(crtc))
> - __array(u32, frame, 3)
> - __array(u32, scanline, 3)
> + __array(u32, frame, I915_MAX_PIPES)
> + __array(u32, scanline, I915_MAX_PIPES)
> __field(enum pipe, pipe)
> ),
>
> @@ -184,8 +185,8 @@ TRACE_EVENT(intel_memory_cxsr,
>
> TP_STRUCT__entry(
> __string(dev, __dev_name_i915(dev_priv))
> - __array(u32, frame, 3)
> - __array(u32, scanline, 3)
> + __array(u32, frame, I915_MAX_PIPES)
> + __array(u32, scanline, I915_MAX_PIPES)
> __field(bool, old)
> __field(bool, new)
> ),
> --
> 2.46.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-gfx
mailing list