Re: [igt-dev] [PATCH i-g-t v3] tests/testdisplay: fix heap overflow

Petri Latvala
2019-03-20T12:17:52Z

On Wed, Mar 20, 2019 at 12:00:13PM +0000, Chris Wilson wrote:
> Quoting Simon Ser (2019-03-20 11:48:57)
> > + argv0 = strdup(argv[0]);
> > + igt_assert(argv0);
> > + exec_path = dirname(argv0);
> > ret = chdir(exec_path);
> > igt_assert_eq(ret, 0);
>
> One should ask Petri if igt_assert_eq() is even legal inside the helper
> (i.e. outside of igt_main and igt_subtest).

*opens testdisplay.c*

*finds main()*

*runs away screaming*

Short answer: It's not legal there

Long answer:
It would be legal there if appropriate steps are taken to ensure IGT
core knows it's a test without subtests. testdisplay, being a dinosaur
that hasn't realized it's pushing up the daisies, doesn't use
igt_simple_main, or call igt_simple_init_parse_opts, or otherwise do
the common things any recently written test is doing.

It's also calling igt_skip_on_simulation in just about the only
possible context where it's not legal.

Note to self: Hurry up with removing all custom main functions.

--
Petri Latvala