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

Chris Wilson chris at chris-wilson.co.uk
Wed Mar 20 11:22:06 UTC 2019


Quoting Simon Ser (2019-03-20 11:15:54)
> Also simplify the code by using dirname(3).
> 
> Signed-off-by: Simon Ser <simon.ser at intel.com>
> ---
>  tests/testdisplay.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/testdisplay.c b/tests/testdisplay.c
> index b3657264..2b26ed1b 100644
> --- a/tests/testdisplay.c
> +++ b/tests/testdisplay.c
> @@ -51,6 +51,7 @@
>  #include <cairo.h>
>  #include <errno.h>
>  #include <getopt.h>
> +#include <libgen.h>
>  #include <math.h>
>  #include <stdint.h>
>  #include <stdbool.h>
> @@ -563,24 +564,14 @@ static gboolean input_event(GIOChannel *source, GIOCondition condition,
>         return TRUE;
>  }
>  
> -static void enter_exec_path( char **argv )
> +static void enter_exec_path(char **argv)
>  {
> -       char *exec_path = NULL;
> -       char *pos = NULL;
> -       short len_path = 0;
> +       char *exec_path;
>         int ret;
>  
> -       len_path = strlen( argv[0] );
> -       exec_path = (char*) malloc(len_path);
> -
> -       memcpy(exec_path, argv[0], len_path);
> -       pos = strrchr(exec_path, '/');
> -       if (pos != NULL)
> -               *(pos+1) = '\0';
> -
> +       exec_path = dirname(argv[0]);

dirname() modifies inplace, so it might not be suitable as presumably we
were copying the argv[0] for a reason :)

exec_path = strcpy(argv[0]);
if (exec_path)
	exec_path = dirname(exec_path);
igt_assert_eq(chdir(exec_path), 0);
free(exec_path);

And if we are not allowed to modify argv, why not say so and make it
const?
-Chris


More information about the igt-dev mailing list