[igt-dev] [PATCH i-g-t] tests/tools_test: Use readlink() properly

[Chris Wilson]

Quoting Arkadiusz Hiler (2019-11-18 12:08:03)
> Turns out that `readlink()` does not NULL-terminate the string and since
> it resides on the stack we may end up with some junk:
> 
>   `/proc/self/exe point to /opt/igt/libexec/igt-gpu-tools/tools_test7<CD>\t<8A>^?`
> 
> That in turn confuses `dirname()` and we end up doing `chdir("/")`,
> which explain the sporadic failures of this test where it was not able
> to locate the tools.

My word, it is even mentioned in the manpage.
 
> Let's zero out the variable first and allow `readlink()` to write at
> most `sizeof()-1` bytes to it, so it is always properly terminated.
> 
> Cc: Petri Latvala <petri.latvala at intel.com>
> Cc: Tomi Sarvela <tomi.p.sarvela at intel.com>
> Issue: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/12
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  tests/tools_test.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/tools_test.c b/tests/tools_test.c
> index e3f73ac2..d486afb9 100644
> --- a/tests/tools_test.c
> +++ b/tests/tools_test.c
> @@ -77,7 +77,8 @@ static bool chdir_to_tools_dir(void)
>         igt_info("Failed to cd to %s\n", TOOLS);
>  
>         /* Try TOOLS and install dir relative to test binary */
> -       if (readlink("/proc/self/exe", path, sizeof(path)) > 0) {
> +       memset(path, 0, sizeof(path)); /* readlink() does not append NULL */

s/NULL/NUL/

> +       if (readlink("/proc/self/exe", path, sizeof(path)-1) > 0) {

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris