Message ID | 20200806143209.4044-1-nixiaoming@huawei.com |
---|---|
State | New |
Headers | show |
Series | stdlib: realpath use malloc replace __alloca to reduce stack overflow risks [BZ #26341] | expand |
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) >
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) + { + __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)
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(-)