Re: [PATCH v3 6/6] stdlib: Add testcase fro BZ #26241

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 20/01/2021 01:33, DJ Delorie wrote:
> 
> Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:
>> -		   tst-setcontext9 tst-bz20544
>> +		   tst-setcontext9 tst-bz20544 tst-canon-bz26341
> 
> New test, ok.
> 
>> +LDLIBS-tst-canon-bz26341 = $(shared-thread-library)
> 
> Ok.
> 
>> diff --git a/stdlib/tst-canon-bz26341.c b/stdlib/tst-canon-bz26341.c
> 
>> +/* Check if realpath does not consume extra stack space based on symlink
>> +   existance in the path (BZ #26341)
> 
> Is this allowed to be two lines?

Good question. GNU code guidelines does not really restrict a number of
line, it is just says

  Also, please write a brief comment at the start of each source file, 
  with the file name and a line or two about the overall purpose of the file.

We do have other file with two line comments, so I think it should be
fine.

> 
>> +   Copyright (C) 2020 Free Software Foundation, Inc.
> 
> Year is wrong now :-)

Ack.

> 
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/param.h>
>> +#include <unistd.h>
> 
> Ok
> 
>> +#define __sysconf sysconf
>> +#include <eloop-threshold.h>
>> +#include <support/check.h>
>> +#include <support/support.h>
>> +#include <support/temp_file.h>
>> +#include <support/xunistd.h>
>> +#include <support/xthread.h>
> 
> Ok
> 
>> +static char *filename;
>> +static size_t filenamelen;
>> +static char *linkname;
>> +
>> +#ifndef PATH_MAX
>> +# define PATH_MAX 1024
>> +#endif
> 
> Ok.
> 
>> +static void
>> +create_link (void)
>> +{
>> +  int fd = create_temp_file ("tst-canon-bz26341", &filename);
>> +  TEST_VERIFY_EXIT (fd != -1);
>> +  xclose (fd);
>> +
>> +  char *prevlink = filename;
>> +  int maxlinks = __eloop_threshold ();
>> +  for (int i = 0; i < maxlinks; i++)
>> +    {
>> +      linkname = xasprintf ("%s%d", filename, i);
>> +      xsymlink (prevlink, linkname);
> 
> linkname -> prevlink -> filename
> 
>> +      add_temp_file (linkname);
>> +      prevlink = linkname;
>> +    }
>> +
>> +  filenamelen = strlen (filename);
>> +}
> 
> On exit, linkname has the last link created.  Needs a comment to that
> effect.

I added prior the link creations.

  /* Create MAXLINKS symbolic links to the temporary filename.
     On exit, linkname has the last link created.  */

> 
> 
>> +static void *
>> +do_realpath (void *arg)
>> +{
>> +  /* Old implementation of realpath allocates a PATH_MAX using alloca
>> +     for each symlink in the path, leading to MAXSYMLINKS times PATH_MAX
>> +     maximum stack usage.
>> +     This stack allocations tries fill the thread allocated stack minus
>> +     both the resolved path (plus some slack) and the realpath (plus some
>> +     slack).
>> +     If realpath uses more than 2 * PATH_MAX plus some slack it will trigger
>> +     a stackoverflow.  */
>> +
>> +  const size_t realpath_usage = 2 * PATH_MAX + 1024;
>> +  const size_t thread_usage = 1 * PATH_MAX + 1024;
>> +  size_t stack_size = support_small_thread_stack_size ()
>> +		      - realpath_usage - thread_usage;
>> +  char stack[stack_size];
>> +  char *resolved = stack + stack_size - thread_usage + 1024;
> 
> This points us at PATH_MAX away from the end of stack[].  Ok.  Also
> forces most of the stack to get used up :-)
> 
>> +  char *p = realpath (linkname, resolved);
> 
> We assume the test will crash if we use more stack than we allocated.
> 
>> +  TEST_VERIFY (p != NULL);
> 
> realpath() must succeed, ok
> 
>> +  TEST_COMPARE_BLOB (resolved, filenamelen, filename, filenamelen);
> 
> And give us the right result, ok
> 
>> +  return NULL;
>> +}
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  create_link ();
>> +
>> +  pthread_t th = xpthread_create (support_small_stack_thread_attribute (),
>> +				  do_realpath, NULL);
>> +  xpthread_join (th);
>> +
>> +  return 0;
>> +}
> 
> Run the test in a thread with a small stack, ok.
> 
>> +#include <support/test-driver.c>
> 
> LGTM with that comment.
> 
> Reviewed-by: DJ Delorie <dj@redhat.com>
> 

[1] https://www.gnu.org/prep/standards/html_node/Comments.html