ping//Re: [PATCH v2] io:nftw/ftw:fix stack overflow when large nopenfd [BZ #26353]

[Xiaoming Ni <nixiaoming@huawei.com>]

ping

On 2020/8/15 15:08, Xiaoming Ni wrote:
> In ftw_startup(), call alloca to apply for a large amount of stack space.
> When descriptors is very large, stack overflow is triggered. BZ #26353
> 
> To fix the problem:
> 1. Set the upper limit of descriptors to getdtablesize().
> 2. Replace alloca() in ftw_startup() with malloc().
> 
> v2:
>    not set errno after malloc fails.
>    add check ftw return value on testcase
>    add more testcase
> v1: https://public-inbox.org/libc-alpha/20200808084640.49174-1-nixiaoming@huawei.com/
> ---
>   io/Makefile      |  3 ++-
>   io/ftw.c         | 15 +++++++++++++--
>   io/tst-bz26353.c | 34 ++++++++++++++++++++++++++++++++++
>   3 files changed, 49 insertions(+), 3 deletions(-)
>   create mode 100644 io/tst-bz26353.c
> 
> diff --git a/io/Makefile b/io/Makefile
> index cf380f3516..0f674c317f 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -74,7 +74,8 @@ tests		:= test-utime test-stat test-stat2 test-lfs tst-getcwd \
>   		   tst-posix_fallocate tst-posix_fallocate64 \
>   		   tst-fts tst-fts-lfs tst-open-tmpfile \
>   		   tst-copy_file_range tst-getcwd-abspath tst-lockf \
> -		   tst-ftw-lnk tst-file_change_detection tst-lchmod
> +		   tst-ftw-lnk tst-file_change_detection tst-lchmod \
> +		   tst-bz26353
>   
>   # Likewise for statx, but we do not need static linking here.
>   tests-internal += tst-statx
> diff --git a/io/ftw.c b/io/ftw.c
> index 8c79d29a9e..f8771a257b 100644
> --- a/io/ftw.c
> +++ b/io/ftw.c
> @@ -643,18 +643,28 @@ ftw_startup (const char *dir, int is_nftw, void *func, int descriptors,
>         __set_errno (ENOENT);
>         return -1;
>       }
> +  if (descriptors > getdtablesize())
> +    {
> +      __set_errno (EINVAL);
> +      return -1;
> +    }
>   
>     data.maxdir = descriptors < 1 ? 1 : descriptors;
>     data.actdir = 0;
> -  data.dirstreams = (struct dir_data **) alloca (data.maxdir
> +  data.dirstreams = (struct dir_data **) malloc (data.maxdir
>   						 * sizeof (struct dir_data *));
> +  if (data.dirstreams == NULL)
> +    return -1;
>     memset (data.dirstreams, '\0', data.maxdir * sizeof (struct dir_data *));
>   
>     /* PATH_MAX is always defined when we get here.  */
>     data.dirbufsize = MAX (2 * strlen (dir), PATH_MAX);
>     data.dirbuf = (char *) malloc (data.dirbufsize);
>     if (data.dirbuf == NULL)
> -    return -1;
> +    {
> +      free (data.dirstreams);
> +      return -1;
> +    }
>     cp = __stpcpy (data.dirbuf, dir);
>     /* Strip trailing slashes.  */
>     while (cp > data.dirbuf + 1 && cp[-1] == '/')
> @@ -805,6 +815,7 @@ ftw_startup (const char *dir, int is_nftw, void *func, int descriptors,
>     __tdestroy (data.known_objects, free);
>     free (data.dirbuf);
>     __set_errno (save_err);
> +  free (data.dirstreams);
>   
>     return result;
>   }
> diff --git a/io/tst-bz26353.c b/io/tst-bz26353.c
> new file mode 100644
> index 0000000000..53e71bfe5e
> --- /dev/null
> +++ b/io/tst-bz26353.c
> @@ -0,0 +1,34 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <ftw.h>
> +#include <sys/stat.h>
> +#include <errno.h>
> +#include <limits.h>
> +
> +int my_func(const char *file, const struct stat *sb ,int flag)
> +{
> +  printf ("%s\n", file);
> +  return 0;
> +}
> +
> +/*Check whether stack overflow occurs*/
> +int do_test(int large_nopenfd)
> +{
> +  int ret = ftw("./tst-bz26353", my_func, large_nopenfd);
> +  printf ("test big num %d, ret=%d errno=%d\n", large_nopenfd, ret, errno);
> +  if (ret != -1 || errno != EINVAL)
> +    return 1;
> +  return 0;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +  int ret = 0;
> +  mkdir ("./tst-bz26353", 0755);
> +  ret += do_test(getdtablesize() + 1);
> +  ret += do_test(8192 * 1024);
> +  ret += do_test(INT_MAX);
> +  rmdir ("./tst-bz26353");
> +  return ret;
> +}
>