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

Ser, Simon
2019-03-21T10:32:28Z

On Wed, 2019-03-20 at 14:17 +0200, Petri Latvala wrote:
> 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.

Hmm. So would you prefer to add an error return value to this function,
or to just continue to use these even if they don't work and fix
everything in a later commit?
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.