Thanks David for the review. Replies inline.

On 10/12/2017 05:22 AM, David Drysdale wrote:
Modulo the minor comment below:

Reviewed-by: David Drysdale <drysd...@google.com>

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

Yeah I found some other instances of this in the test (specifically the calls to realpath() at the top of run_tests() which alloc their own buffer) and figured for a small unit test like this it had been decided it wasn't worth freeing stuff. I'll tidy this up anyway though.


+
+               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).

Will fix.

thanks,
Steve

Reply via email to