lkml.org 
[lkml]   [2017]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] selftests/exec: include cwd in long path calculation
From
Date
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

\
 
 \ /
  Last update: 2017-10-12 19:38    [W:0.042 / U:2.448 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site