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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHse=S_waBFB5pZVqVJp4wepm1K_LXqBY58YB2SOHX=O4xbV8w@mail.gmail.com>
Date:   Thu, 12 Oct 2017 13:22:56 +0100
From:   David Drysdale <drysdale@...gle.com>
To:     Steve Muckle <smuckle.linux@...il.com>
Cc:     Shuah Khan <shuah@...nel.org>, linux-kselftest@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] selftests/exec: include cwd in long path calculation

Modulo the minor comment below:

Reviewed-by: David Drysdale <drysdale@...gle.com>

On Thu, Oct 12, 2017 at 1:40 AM, Steve Muckle <smuckle.linux@...il.com> 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@...il.com>
> ---
>  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
>