From: Gustavo Sousa <gustavo.sousa@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Matt Roper <matthew.d.roper@intel.com>
Subject: [PATCH v2 1/5] drm/i915/display: Fix out-of-bounds access in pipe-related tracepoints
Date: Mon, 23 Sep 2024 16:02:50 -0300 [thread overview]
Message-ID: <20240923190324.83013-2-gustavo.sousa@intel.com> (raw)
In-Reply-To: <20240923190324.83013-1-gustavo.sousa@intel.com>
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.
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
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 fc28d34b5eef..e70c015a09a1 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_display(display))
- __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.1
next prev parent reply other threads:[~2024-09-23 19:03 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-23 19:02 [PATCH v2 0/5] Miscelaneous fixes for display tracepoints Gustavo Sousa
2024-09-23 19:02 ` Gustavo Sousa [this message]
2024-09-23 19:02 ` [PATCH v2 2/5] drm/i915/display: Zero-initialize frame/scanline counts in tracepoints Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 3/5] drm/i915/display: Store pipe name in trace events Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 4/5] drm/i915/display: Do not use ids from enum pipe in TP_printk() Gustavo Sousa
2024-09-23 19:02 ` [PATCH v2 5/5] drm/i915/display: Cover all possible pipes " Gustavo Sousa
2024-09-23 19:23 ` Ville Syrjälä
2024-09-23 20:06 ` Gustavo Sousa
2024-09-23 20:18 ` Ville Syrjälä
2024-09-23 20:47 ` Gustavo Sousa
2024-09-23 20:48 ` Gustavo Sousa
2024-09-23 21:08 ` Ville Syrjälä
2024-09-26 19:26 ` ✗ Fi.CI.CHECKPATCH: warning for Miscelaneous fixes for display tracepoints (rev2) Patchwork
2024-09-26 19:28 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-10-04 11:12 ` Gustavo Sousa
2024-10-04 11:33 ` Jani Nikula
2024-10-04 11:43 ` Gustavo Sousa
2024-10-04 12:03 ` Saarinen, Jani
2024-10-04 12:09 ` Saarinen, Jani
2024-10-08 19:46 ` ✗ Fi.CI.CHECKPATCH: warning for Miscelaneous fixes for display tracepoints (rev3) Patchwork
2024-10-08 19:55 ` ✗ Fi.CI.BAT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240923190324.83013-2-gustavo.sousa@intel.com \
--to=gustavo.sousa@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.d.roper@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox