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

[DJ Delorie <dj@redhat.com>]

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?

> +   Copyright (C) 2020 Free Software Foundation, Inc.

Year is wrong now :-)

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


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