Re: [PATCH 0/6] posix: Do not recurse once per pattern component in glob (BZ #34453)

Adhemerval Zanella <[email protected]> writes:

> glob calls itself recursively once per directory component and once per
> brace expression, so stack usage grows with the length of the pattern
> and a few thousand components overflow a default 8 MiB stack before
> glob can answer (BZ #34453).
>
> The last patch replaces the recursion with heap-allocated state, the
> same approach used for the recent ftw fix (commit 418581126ac, "io:
> ftw: Use state stack instead of recursion (BZ 33882)").  The directory
> components are collected into an array and matched iteratively from
> left to right, and the brace expansions are walked with an explicit
> stack.
>
> The patches before it remove the alloca usage from glob.  Besides
> simplifying the recursion removal, this makes glibc and gnulib run the
> same code: gnulib builds this file with __libc_use_alloca defined to
> false, so the alloca paths were compiled out there and only glibc ever
> executed them.  The alloca accounting is also a tricky and misleading
> API, it only counts the buffers explicitly allocated through
> alloca_account, not the rest of each frame, and every recursive call
> started again with a fresh budget, so it never bounded the actual
> stack usage.
>
> Checked on x86_64-linux-gnu, aarch64-linux-gnu, and i686-linux-gnu.
> The file was also built and tested as gnulib's glob replacement.
>
> Adhemerval Zanella (6):
>   posix: Use malloc instead of alloca for the glob directory name
>   posix: Move the glob home directory lookups out of __glob
>   posix: Use malloc instead of alloca for the glob user name
>   posix: Use malloc instead of alloca for the glob brace expansion
>   posix: Remove the alloca uses from glob_in_dir
>   posix: Do not recurse once per pattern component in glob [BZ #34453]
>
>  posix/Makefile           |    1 +
>  posix/glob.c             | 1043 ++++++++++++++++++++++++++------------
>  posix/tst-glob-bz34453.c |  116 +++++
>  posix/tst-glob-tilde.c   |   19 +
>  4 files changed, 847 insertions(+), 332 deletions(-)
>  create mode 100644 posix/tst-glob-bz34453.c

The general idea of the change seems good to me, although I haven't
reviewed the patches yet.

Just want to mention that my feeling is that this isn't a security issue
as the bug report says. I don't think any sensitive services are
allowing users to pass arbitrary number of wildcards to glob.

Collin