[PATCH 1/6] iconv: Remove alloca use in gconv-modules configuration parsing
DJ Delorie
dj@redhat.com
Tue Jun 22 03:17:44 GMT 2021
Siddhesh Poyarekar <siddhesh@sourceware.org> writes:
> The alloca sizes ought to be constrained to PATH_MAX, but replace them
> with dynamic allocation to be safe. A static PATH_MAX array would
> have worked too but Hurd does not have PATH_MAX and the code path is
> not hot enough to micro-optimise this allocation. Revisit if any of
> those realities change.
> ---
> iconv/gconv_conf.c | 17 +++++++++--------
> iconv/iconvconfig.c | 17 +++++++++++------
> 2 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/iconv/gconv_conf.c b/iconv/gconv_conf.c
> index c8ad8099a4..3f2cef255b 100644
> --- a/iconv/gconv_conf.c
> +++ b/iconv/gconv_conf.c
> @@ -559,15 +559,15 @@ __gconv_read_conf (void)
>
> for (cnt = 0; __gconv_path_elem[cnt].name != NULL; ++cnt)
> {
> -#define BUF_LEN elem_len + sizeof (gconv_conf_dirname)
> -
> const char *elem = __gconv_path_elem[cnt].name;
> size_t elem_len = __gconv_path_elem[cnt].len;
> - char *buf;
>
> /* No slash needs to be inserted between elem and gconv_conf_filename;
> elem already ends in a slash. */
> - buf = alloca (BUF_LEN);
> + char *buf = malloc (elem_len + sizeof (gconv_conf_dirname));
> + if (buf == NULL)
> + continue;
> +
> char *cp = __mempcpy (__mempcpy (buf, elem, elem_len),
> gconv_conf_filename, sizeof (gconv_conf_filename));
>
replaces alloca with malloc; sizes are the same. Ok.
> @@ -596,15 +596,16 @@ __gconv_read_conf (void)
> if (len > strlen (suffix)
> && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)
> {
> - /* LEN <= PATH_MAX so this alloca is not unbounded. */
> - char *conf = alloca (BUF_LEN + len + 1);
> - cp = stpcpy (conf, buf);
> - sprintf (cp, "/%s", ent->d_name);
> + char *conf;
> + if (__asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)
> + continue;
> read_conf_file (conf, elem, elem_len, &modules, &nmodules);
> + free (conf);
allocated by asprintf, free'd here. Ok.
> }
> }
> __closedir (confdir);
> }
> + free (buf);
Matched free, OK.
> }
> #endif
>
> diff --git a/iconv/iconvconfig.c b/iconv/iconvconfig.c
> index b2a868919c..c9607fb645 100644
> --- a/iconv/iconvconfig.c
> +++ b/iconv/iconvconfig.c
> @@ -712,7 +712,6 @@ handle_file (const char *dir, const char *infile)
> static int
> handle_dir (const char *dir)
> {
> -#define BUF_LEN prefix_len + dirlen + sizeof "gconv-modules.d"
> char *cp;
> size_t dirlen = strlen (dir);
> bool found = false;
> @@ -726,7 +725,10 @@ handle_dir (const char *dir)
> }
>
> /* First, look for a gconv-modules file. */
> - char buf[BUF_LEN];
> + char *buf = malloc (prefix_len + dirlen + sizeof "gconv-modules.d");
> + if (buf == NULL)
> + goto out;
> +
Likewise, ok.
> cp = buf;
> if (dir[0] == '/')
> cp = mempcpy (cp, prefix, prefix_len);
> @@ -756,16 +758,19 @@ handle_dir (const char *dir)
> if (len > strlen (suffix)
> && strcmp (ent->d_name + len - strlen (suffix), suffix) == 0)
> {
> - /* LEN <= PATH_MAX so this alloca is not unbounded. */
> - char *conf = alloca (BUF_LEN + len + 1);
> - cp = stpcpy (conf, buf);
> - sprintf (cp, "/%s", ent->d_name);
> + char *conf;
> + if (asprintf (&conf, "%s/%s", buf, ent->d_name) < 0)
> + continue;
> found |= handle_file (dir, conf);
> + free (conf);
Likewise, ok.
> }
> }
> closedir (confdir);
> }
>
> + free (buf);
> +
> +out:
Freed, ok.
> if (!found)
> {
> error (0, errno, "failed to open gconv configuration files in `%s'",
LGTM.
Reviewed-by: DJ Delorie <dj@redhat.com>
More information about the Libc-alpha
mailing list