Messages in this thread | | | Subject | Re: [PATCH] selftests/exec: include cwd in long path calculation | From | Steve Muckle <> | Date | Thu, 12 Oct 2017 10:36:55 -0700 |
| |
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 <drysdale@google.com> > > On Thu, Oct 12, 2017 at 1:40 AM, Steve Muckle <smuckle.linux@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.linux@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
| |