Message ID | 20230913160042.326350-1-josimmon@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v3] argp-help: Get rid of alloca. | expand |
On 13/09/23 13:00, Joe Simmons-Talbott wrote: > Replace alloca with malloc and a scratch_buffer to avoid potential stack > overflow. > > Checked on x86_64-linux-gnu > --- > argp/argp-help.c | 40 ++++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > > diff --git a/argp/argp-help.c b/argp/argp-help.c > index d019ed58d2..54a15a8233 100644 > --- a/argp/argp-help.c > +++ b/argp/argp-help.c > @@ -25,21 +25,7 @@ > #include <config.h> > #endif > > -/* AIX requires this to be the first thing in the file. */ > -#ifndef __GNUC__ > -# if HAVE_ALLOCA_H || defined _LIBC > -# include <alloca.h> > -# else > -# ifdef _AIX > -#pragma alloca > -# else > -# ifndef alloca /* predefined by HP cc +Olibcalls */ > -char *alloca (); > -# endif > -# endif > -# endif > -#endif > - > +#include <scratch_buffer.h> > #include <stdbool.h> > #include <stddef.h> > #include <stdlib.h> > @@ -1450,8 +1436,14 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) > { > unsigned nentries; > struct hol_entry *entry; > - char *short_no_arg_opts = alloca (strlen (hol->short_options) + 1); > - char *snao_end = short_no_arg_opts; > + char *short_no_arg_opts; > + char *snao_end; > + > + short_no_arg_opts = malloc (strlen (hol->short_options) + 1); > + assert (short_no_arg_opts != NULL); > + > + snao_end = short_no_arg_opts; > + > > /* First we put a list of short options without arguments. */ > for (entry = hol->entries, nentries = hol->num_entries > @@ -1478,6 +1470,8 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) > ; entry++, nentries--) > hol_entry_long_iterate (entry, usage_long_opt, > entry->argp->argp_domain, stream); > + > + free (short_no_arg_opts); > } > } > > @@ -1698,7 +1692,15 @@ _help (const struct argp *argp, const struct argp_state *state, FILE *stream, > { > int first_pattern = 1, more_patterns; > size_t num_pattern_levels = argp_args_levels (argp); > - char *pattern_levels = alloca (num_pattern_levels); > + struct scratch_buffer buf; > + scratch_buffer_init (&buf); > + char *pattern_levels; > + bool result; > + > + result = scratch_buffer_set_array_size (&buf, 1, num_pattern_levels); > + assert (result); > + > + pattern_levels = buf.data; > > memset (pattern_levels, 0, num_pattern_levels); > Use malloc here as well. > @@ -1746,6 +1748,8 @@ _help (const struct argp *argp, const struct argp_state *state, FILE *stream, > first_pattern = 0; > } > while (more_patterns); > + > + scratch_buffer_free (&buf); > } > > if (flags & ARGP_HELP_PRE_DOC)
On Wed, Sep 13, 2023 at 04:45:21PM -0300, Adhemerval Zanella Netto wrote: > > > On 13/09/23 13:00, Joe Simmons-Talbott wrote: > > Replace alloca with malloc and a scratch_buffer to avoid potential stack > > overflow. > > > > Checked on x86_64-linux-gnu > > --- > > argp/argp-help.c | 40 ++++++++++++++++++++++------------------ > > 1 file changed, 22 insertions(+), 18 deletions(-) > > > > diff --git a/argp/argp-help.c b/argp/argp-help.c > > index d019ed58d2..54a15a8233 100644 > > --- a/argp/argp-help.c > > +++ b/argp/argp-help.c > > @@ -25,21 +25,7 @@ > > #include <config.h> > > #endif > > > > -/* AIX requires this to be the first thing in the file. */ > > -#ifndef __GNUC__ > > -# if HAVE_ALLOCA_H || defined _LIBC > > -# include <alloca.h> > > -# else > > -# ifdef _AIX > > -#pragma alloca > > -# else > > -# ifndef alloca /* predefined by HP cc +Olibcalls */ > > -char *alloca (); > > -# endif > > -# endif > > -# endif > > -#endif > > - > > +#include <scratch_buffer.h> > > #include <stdbool.h> > > #include <stddef.h> > > #include <stdlib.h> > > @@ -1450,8 +1436,14 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) > > { > > unsigned nentries; > > struct hol_entry *entry; > > - char *short_no_arg_opts = alloca (strlen (hol->short_options) + 1); > > - char *snao_end = short_no_arg_opts; > > + char *short_no_arg_opts; > > + char *snao_end; > > + > > + short_no_arg_opts = malloc (strlen (hol->short_options) + 1); > > + assert (short_no_arg_opts != NULL); > > + > > + snao_end = short_no_arg_opts; > > + > > > > /* First we put a list of short options without arguments. */ > > for (entry = hol->entries, nentries = hol->num_entries > > @@ -1478,6 +1470,8 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) > > ; entry++, nentries--) > > hol_entry_long_iterate (entry, usage_long_opt, > > entry->argp->argp_domain, stream); > > + > > + free (short_no_arg_opts); > > } > > } > > > > @@ -1698,7 +1692,15 @@ _help (const struct argp *argp, const struct argp_state *state, FILE *stream, > > { > > int first_pattern = 1, more_patterns; > > size_t num_pattern_levels = argp_args_levels (argp); > > - char *pattern_levels = alloca (num_pattern_levels); > > + struct scratch_buffer buf; > > + scratch_buffer_init (&buf); > > + char *pattern_levels; > > + bool result; > > + > > + result = scratch_buffer_set_array_size (&buf, 1, num_pattern_levels); > > + assert (result); > > + > > + pattern_levels = buf.data; > > > > memset (pattern_levels, 0, num_pattern_levels); > > > > > Use malloc here as well. Done in v4.
diff --git a/argp/argp-help.c b/argp/argp-help.c index d019ed58d2..54a15a8233 100644 --- a/argp/argp-help.c +++ b/argp/argp-help.c @@ -25,21 +25,7 @@ #include <config.h> #endif -/* AIX requires this to be the first thing in the file. */ -#ifndef __GNUC__ -# if HAVE_ALLOCA_H || defined _LIBC -# include <alloca.h> -# else -# ifdef _AIX -#pragma alloca -# else -# ifndef alloca /* predefined by HP cc +Olibcalls */ -char *alloca (); -# endif -# endif -# endif -#endif - +#include <scratch_buffer.h> #include <stdbool.h> #include <stddef.h> #include <stdlib.h> @@ -1450,8 +1436,14 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) { unsigned nentries; struct hol_entry *entry; - char *short_no_arg_opts = alloca (strlen (hol->short_options) + 1); - char *snao_end = short_no_arg_opts; + char *short_no_arg_opts; + char *snao_end; + + short_no_arg_opts = malloc (strlen (hol->short_options) + 1); + assert (short_no_arg_opts != NULL); + + snao_end = short_no_arg_opts; + /* First we put a list of short options without arguments. */ for (entry = hol->entries, nentries = hol->num_entries @@ -1478,6 +1470,8 @@ hol_usage (struct hol *hol, argp_fmtstream_t stream) ; entry++, nentries--) hol_entry_long_iterate (entry, usage_long_opt, entry->argp->argp_domain, stream); + + free (short_no_arg_opts); } } @@ -1698,7 +1692,15 @@ _help (const struct argp *argp, const struct argp_state *state, FILE *stream, { int first_pattern = 1, more_patterns; size_t num_pattern_levels = argp_args_levels (argp); - char *pattern_levels = alloca (num_pattern_levels); + struct scratch_buffer buf; + scratch_buffer_init (&buf); + char *pattern_levels; + bool result; + + result = scratch_buffer_set_array_size (&buf, 1, num_pattern_levels); + assert (result); + + pattern_levels = buf.data; memset (pattern_levels, 0, num_pattern_levels); @@ -1746,6 +1748,8 @@ _help (const struct argp *argp, const struct argp_state *state, FILE *stream, first_pattern = 0; } while (more_patterns); + + scratch_buffer_free (&buf); } if (flags & ARGP_HELP_PRE_DOC)