[igt-dev] [PATCH i-g-t] lib/kms: Range check pipe before lookup

[Chris Wilson]

Quoting Petri Latvala (2020-11-19 08:53:04)
> On Thu, Nov 19, 2020 at 08:35:59AM +0000, Chris Wilson wrote:
> > Make sure the lookup index is within the range of the table before
> > accessing.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  lib/igt_kms.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index e5d8e82c9..9175e50fe 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1902,7 +1902,7 @@ static void igt_fill_display_format_mod(igt_display_t *display);
> >   */
> >  void igt_require_pipe(igt_display_t *display, enum pipe pipe)
> >  {
> > -     igt_skip_on_f(!display->pipes[pipe].enabled,
> > +     igt_skip_on_f(pipe >= display->n_pipes || !display->pipes[pipe].enabled,
> >                       "Pipe %s does not exist or not enabled\n",
> >                       kmstest_pipe_name(pipe));
> 
> Who's calling this with an invalid 'pipe' value? display->pipes size
> is no longer display->n_pipes, right?

https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9356/shard-skl8/igt@kms_pipe_crc_basic@disable-crc-after-crtc-pipe-b.html
Starting subtest: disable-crc-after-crtc-pipe-B
Received signal SIGSEGV.
Stack trace:
 #0 [fatal_sig_handler+0xd6]
 #1 [killpg+0x40]
 #2 [igt_output_get_mode+0x0]
 #3 [__real_main261+0x629]
 #4 [main+0x27]
 #5 [__libc_start_main+0xe7]
 #6 [_start+0x2a]
Subtest disable-crc-after-crtc-pipe-B: CRASH (0.087s)

Suggests that the output is NULL/garbage, so I was looking for ideas as
to how we end up dying there. The loop in test_read_crc() doesn't look
susceptible to corrupting the local variable, so I guess the output was
suspect to begin with.
-Chris