Re: [PATCH] selftests/exec: include cwd in long path calculation

From: David Drysdale
Date: Thu Oct 12 2017 - 08:23:24 EST


Modulo the minor comment below:

Reviewed-by: David Drysdale <drysdale@xxxxxxxxxx>

On Thu, Oct 12, 2017 at 1:40 AM, Steve Muckle <smuckle.linux@xxxxxxxxx> wrote:
> When creating a pathname close to PATH_MAX to test execveat, factor in
> the current working directory path otherwise we end up with an absolute
> path that is longer than PATH_MAX. While execveat() may succeed, subsequent
> calls to the kernel from the runtime environment which are required to
> successfully execute the test binary/script may fail because of this.
>
> To keep the semantics of the test the same, rework the relative pathname
> part of the test to be relative to the root directory so it isn't
> decreased by the length of the current working directory path.
>
> Signed-off-by: Steve Muckle <smuckle.linux@xxxxxxxxx>
> ---
> tools/testing/selftests/exec/execveat.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
> index 8d5d1d2ee7c1..5edc609c778b 100644
> --- a/tools/testing/selftests/exec/execveat.c
> +++ b/tools/testing/selftests/exec/execveat.c
> @@ -147,7 +147,7 @@ static void exe_cp(const char *src, const char *dest)
> }
>
> #define XX_DIR_LEN 200
> -static int check_execveat_pathmax(int dot_dfd, const char *src, int is_script)
> +static int check_execveat_pathmax(int root_dfd, const char *src, int is_script)
> {
> int fail = 0;
> int ii, count, len;
> @@ -156,15 +156,24 @@ static int check_execveat_pathmax(int dot_dfd, const char *src, int is_script)
>
> if (*longpath == '\0') {
> /* Create a filename close to PATH_MAX in length */
> + char *cwd = getcwd(NULL, 0);

cwd never gets freed, but that's presumably not a problem unless these
tests ever get run under some kind of leak checker.

> +
> + if (!cwd) {
> + printf("Failed to getcwd(), errno=%d (%s)\n",
> + errno, strerror(errno));
> + return 2;
> + }
> + strcpy(longpath, cwd);
> + strcat(longpath, "/");
> memset(longname, 'x', XX_DIR_LEN - 1);
> longname[XX_DIR_LEN - 1] = '/';
> longname[XX_DIR_LEN] = '\0';
> - count = (PATH_MAX - 3) / XX_DIR_LEN;
> + count = (PATH_MAX - 3 - strlen(cwd)) / XX_DIR_LEN;
> for (ii = 0; ii < count; ii++) {
> strcat(longpath, longname);
> mkdir(longpath, 0755);
> }
> - len = (PATH_MAX - 3) - (count * XX_DIR_LEN);
> + len = (PATH_MAX - 3 - strlen(cwd)) - (count * XX_DIR_LEN);
> if (len <= 0)
> len = 1;
> memset(longname, 'y', len);

There's a missing comment update around here-ish:
* Execute as a long pathname relative to ".".
(should now be 'relative to "/"' I think).

> @@ -200,10 +209,10 @@ static int check_execveat_pathmax(int dot_dfd, const char *src, int is_script)
> * the exit status shall be 126."), so allow either.
> */
> if (is_script)
> - fail += check_execveat_invoked_rc(dot_dfd, longpath, 0,
> + fail += check_execveat_invoked_rc(root_dfd, longpath + 1, 0,
> 127, 126);
> else
> - fail += check_execveat(dot_dfd, longpath, 0);
> + fail += check_execveat(root_dfd, longpath + 1, 0);
>
> return fail;
> }
> @@ -218,6 +227,7 @@ static int run_tests(void)
> int subdir_dfd_ephemeral = open_or_die("subdir.ephemeral",
> O_DIRECTORY|O_RDONLY);
> int dot_dfd = open_or_die(".", O_DIRECTORY|O_RDONLY);
> + int root_dfd = open_or_die("/", O_DIRECTORY|O_RDONLY);
> int dot_dfd_path = open_or_die(".", O_DIRECTORY|O_RDONLY|O_PATH);
> int dot_dfd_cloexec = open_or_die(".", O_DIRECTORY|O_RDONLY|O_CLOEXEC);
> int fd = open_or_die("execveat", O_RDONLY);
> @@ -353,8 +363,8 @@ static int run_tests(void)
> /* Attempt to execute relative to non-directory => ENOTDIR */
> fail += check_execveat_fail(fd, "execveat", 0, ENOTDIR);
>
> - fail += check_execveat_pathmax(dot_dfd, "execveat", 0);
> - fail += check_execveat_pathmax(dot_dfd, "script", 1);
> + fail += check_execveat_pathmax(root_dfd, "execveat", 0);
> + fail += check_execveat_pathmax(root_dfd, "script", 1);
> return fail;
> }
>
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>