stdlib: realpath use malloc replace __alloca to reduce stack overflow risks [BZ #26341]
Commit Message
Realpath() cyclically invokes __alloca() when processing soft link files,
which may consume 164 KB stack space.
Therefore, replace __alloca with malloc to reduce stack overflow risks
Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
---
stdlib/canonicalize.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
Comments
On 06/08/2020 11:32, Xiaoming Ni wrote:
> Realpath() cyclically invokes __alloca() when processing soft link files,
> which may consume 164 KB stack space.
> Therefore, replace __alloca with malloc to reduce stack overflow risks
>
> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
We do not use SCO, but rather copyright assignments (Carlos can check if your
is ok with FSF).
If possible could you provide a testcase? Maybe by limiting the stack with
getrlimit and using the example provided in the bugzilla? Patch look ok,
just formatting style nits.
As a side note, for Linux we could simplify the realpath implementation
a *lot* with limited stack size and no malloc allocation we could remove
the GNU extension to return prefix of path that is not readable or does
not exist if resolved_path is not NULL (is a GNU extension that came
from the implementation itself that works directly on the buffer).
> ---
> stdlib/canonicalize.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
> index cbd885a3c5..d3130d81f0 100644
> --- a/stdlib/canonicalize.c
> +++ b/stdlib/canonicalize.c
> @@ -163,27 +163,46 @@ __realpath (const char *name, char *resolved)
>
> if (S_ISLNK (st.st_mode))
> {
> - char *buf = __alloca (path_max);
> + char *buf = malloc (path_max);
> size_t len;
>
> + if (!buf)
Use explicit check (buf != NULL).
> + {
> + __set_errno (ENOMEM);
> + goto error;
> + }
> +
> if (++num_links > __eloop_threshold ())
> {
> __set_errno (ELOOP);
> + free(buf);
Space after free.
> goto error;
> }
>
> n = __readlink (rpath, buf, path_max - 1);
> if (n < 0)
> - goto error;
> + {
> + free(buf);
Ditto.
> + goto error;
> + }
> buf[n] = '\0';
>
> if (!extra_buf)
> - extra_buf = __alloca (path_max);
> + {
> + extra_buf = malloc (path_max);
> + if (!extra_buf)
Use explicit check (extra_buf != NULL).
> + {
> + free(buf);
Space after free.
> + __set_errno (ENOMEM);
> + goto error;
> + }
> + }
>
> len = strlen (end);
> if (path_max - n <= len)
> {
> __set_errno (ENAMETOOLONG);
> + free(buf);
Ditto.
> goto error;
> }
>
> @@ -197,6 +216,7 @@ __realpath (const char *name, char *resolved)
> /* Back up to previous component, ignore if at root already: */
> if (dest > rpath + 1)
> while ((--dest)[-1] != '/');
> + free(buf);
Ditto.
> }
> else if (!S_ISDIR (st.st_mode) && *end != '\0')
> {
> @@ -210,12 +230,14 @@ __realpath (const char *name, char *resolved)
> *dest = '\0';
>
> assert (resolved == NULL || resolved == rpath);
> + free(extra_buf);
> return rpath;
>
> error:
> assert (resolved == NULL || resolved == rpath);
> if (resolved == NULL)
> free (rpath);
> + free(extra_buf);
Space after free.
> return NULL;
> }
> libc_hidden_def (__realpath)
>
@@ -163,27 +163,46 @@ __realpath (const char *name, char *resolved)
if (S_ISLNK (st.st_mode))
{
- char *buf = __alloca (path_max);
+ char *buf = malloc (path_max);
size_t len;
+ if (!buf)
+ {
+ __set_errno (ENOMEM);
+ goto error;
+ }
+
if (++num_links > __eloop_threshold ())
{
__set_errno (ELOOP);
+ free(buf);
goto error;
}
n = __readlink (rpath, buf, path_max - 1);
if (n < 0)
- goto error;
+ {
+ free(buf);
+ goto error;
+ }
buf[n] = '\0';
if (!extra_buf)
- extra_buf = __alloca (path_max);
+ {
+ extra_buf = malloc (path_max);
+ if (!extra_buf)
+ {
+ free(buf);
+ __set_errno (ENOMEM);
+ goto error;
+ }
+ }
len = strlen (end);
if (path_max - n <= len)
{
__set_errno (ENAMETOOLONG);
+ free(buf);
goto error;
}
@@ -197,6 +216,7 @@ __realpath (const char *name, char *resolved)
/* Back up to previous component, ignore if at root already: */
if (dest > rpath + 1)
while ((--dest)[-1] != '/');
+ free(buf);
}
else if (!S_ISDIR (st.st_mode) && *end != '\0')
{
@@ -210,12 +230,14 @@ __realpath (const char *name, char *resolved)
*dest = '\0';
assert (resolved == NULL || resolved == rpath);
+ free(extra_buf);
return rpath;
error:
assert (resolved == NULL || resolved == rpath);
if (resolved == NULL)
free (rpath);
+ free(extra_buf);
return NULL;
}
libc_hidden_def (__realpath)