* [PATCH] selftests/exec: include cwd in long path calculation
@ 2017-10-12 0:40 Steve Muckle
2017-10-12 12:22 ` David Drysdale
0 siblings, 1 reply; 3+ messages in thread
From: Steve Muckle @ 2017-10-12 0:40 UTC (permalink / raw)
To: Shuah Khan; +Cc: David Drysdale, linux-kselftest, linux-kernel, Steve Muckle
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);
+
+ 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);
@@ -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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] selftests/exec: include cwd in long path calculation
2017-10-12 0:40 [PATCH] selftests/exec: include cwd in long path calculation Steve Muckle
@ 2017-10-12 12:22 ` David Drysdale
2017-10-12 17:36 ` Steve Muckle
0 siblings, 1 reply; 3+ messages in thread
From: David Drysdale @ 2017-10-12 12:22 UTC (permalink / raw)
To: Steve Muckle; +Cc: Shuah Khan, linux-kselftest, linux-kernel@vger.kernel.org
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.
> +
> + 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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] selftests/exec: include cwd in long path calculation
2017-10-12 12:22 ` David Drysdale
@ 2017-10-12 17:36 ` Steve Muckle
0 siblings, 0 replies; 3+ messages in thread
From: Steve Muckle @ 2017-10-12 17:36 UTC (permalink / raw)
To: David Drysdale; +Cc: Shuah Khan, linux-kselftest, linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-12 17:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-12 0:40 [PATCH] selftests/exec: include cwd in long path calculation Steve Muckle
2017-10-12 12:22 ` David Drysdale
2017-10-12 17:36 ` Steve Muckle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.