[v2] stdlib: realpath use malloc replace __alloca to reduce stack overflow risks [BZ #26341]

Message ID 20200807101601.61670-1-nixiaoming@huawei.com
State Superseded
Headers
Series [v2] stdlib: realpath use malloc replace __alloca to reduce stack overflow risks [BZ #26341] |

Commit Message

Xiaoming Ni Aug. 7, 2020, 10:16 a.m. UTC
  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

v2: Avoid repeated malloc and free operations. and add testcase
v1: https://patches-gcc.linaro.org/patch/39851/

Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
---
 stdlib/Makefile       |  3 +-
 stdlib/canonicalize.c | 25 +++++++++++++--
 stdlib/tst-bz26341.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 3 deletions(-)
 create mode 100644 stdlib/tst-bz26341.c
  

Comments

Adhemerval Zanella Aug. 7, 2020, 7:43 p.m. UTC | #1
On 07/08/2020 07:16, 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
> 
> v2: Avoid repeated malloc and free operations. and add testcase
> v1: https://patches-gcc.linaro.org/patch/39851/
> 
> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>

Again, we do not use DCO, but rather Copyright assignment. 

Paul, it might be something to be fixed on gnulib since I noted that both
gpl and lgpl code uses malloca (which calls alloca if the header is present).

Some comments below regarding testing.

> ---
>  stdlib/Makefile       |  3 +-
>  stdlib/canonicalize.c | 25 +++++++++++++--
>  stdlib/tst-bz26341.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 3 deletions(-)
>  create mode 100644 stdlib/tst-bz26341.c
> 
> diff --git a/stdlib/Makefile b/stdlib/Makefile
> index 4615f6dfe7..bfdd9036b1 100644
> --- a/stdlib/Makefile
> +++ b/stdlib/Makefile
> @@ -87,7 +87,7 @@ tests		:= tst-strtol tst-strtod testmb testrand testsort testdiv   \
>  		   tst-makecontext-align test-bz22786 tst-strtod-nan-sign \
>  		   tst-swapcontext1 tst-setcontext4 tst-setcontext5 \
>  		   tst-setcontext6 tst-setcontext7 tst-setcontext8 \
> -		   tst-setcontext9 tst-bz20544
> +		   tst-setcontext9 tst-bz20544 tst-bz26341
>  
>  tests-internal	:= tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
>  		   tst-tls-atexit tst-tls-atexit-nodelete

Ok.

> @@ -98,6 +98,7 @@ ifeq ($(build-hardcoded-path-in-tests),yes)
>  tests += tst-empty-env
>  endif
>  
> +LDLIBS-tst-bz26341 = -lpthread

This needs to be $(shared-thread-library).

>  LDLIBS-test-atexit-race = $(shared-thread-library)
>  LDLIBS-test-at_quick_exit-race = $(shared-thread-library)
>  LDLIBS-test-cxa_atexit-race = $(shared-thread-library)
> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
> index cbd885a3c5..c02a8a5800 100644
> --- a/stdlib/canonicalize.c
> +++ b/stdlib/canonicalize.c
> @@ -46,6 +46,7 @@ __realpath (const char *name, char *resolved)
>    const char *start, *end, *rpath_limit;
>    long int path_max;
>    int num_links = 0;
> +  char *buf = NULL;
>  
>    if (name == NULL)
>      {

Ok.

> @@ -163,9 +164,18 @@ __realpath (const char *name, char *resolved)
>  
>  	  if (S_ISLNK (st.st_mode))
>  	    {
> -	      char *buf = __alloca (path_max);
>  	      size_t len;
>  
> +        if (buf == NULL)
> +          {
> +            buf = malloc (path_max);
> +            if (buf == NULL)
> +              {
> +                __set_errno (ENOMEM);
> +                goto error;
> +              }
> +          }
> +
>  	      if (++num_links > __eloop_threshold ())
>  		{
>  		  __set_errno (ELOOP);

Ok.

> @@ -178,7 +188,14 @@ __realpath (const char *name, char *resolved)
>  	      buf[n] = '\0';
>  
>  	      if (!extra_buf)
> -		extra_buf = __alloca (path_max);
> +          {
> +            extra_buf = malloc (path_max);
> +            if (extra_buf == NULL)
> +              {
> +                __set_errno (ENOMEM);
> +                goto error;
> +              }
> +          }
>  
>  	      len = strlen (end);
>  	      if (path_max - n <= len)

Ok.

> @@ -210,12 +227,16 @@ __realpath (const char *name, char *resolved)
>    *dest = '\0';
>  
>    assert (resolved == NULL || resolved == rpath);
> +  free(extra_buf);
> +  free(buf);
>    return rpath;
>  
>  error:
>    assert (resolved == NULL || resolved == rpath);
>    if (resolved == NULL)
>      free (rpath);
> +  free (extra_buf);
> +  free (buf);
>    return NULL;
>  }
>  libc_hidden_def (__realpath)

Ok.

> diff --git a/stdlib/tst-bz26341.c b/stdlib/tst-bz26341.c
> new file mode 100644
> index 0000000000..0fe095b7d1
> --- /dev/null
> +++ b/stdlib/tst-bz26341.c
> @@ -0,0 +1,73 @@

This test need the Copyright header and to be properly indented using glibc
code guideline.

Use it need to use the libsupport (check support/README-testing.c and other
tests that use '#include <support/test-driver.c>'.


> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <limits.h>
> +#include <sys/resource.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <string.h>
> +#include <pthread.h>
> +
> +int creat_link(void)
> +{
> +	int i;
> +	int fd;
> +	char fname[2][16] = {0};
> +	char *p1 = fname[0];
> +	char *p2 = fname[1];
> +	strcpy(p1, "f0");
> +	fd = open(p1, O_RDONLY|O_CREAT, 0444);
> +	close(fd);
> +
> +	for (i = 0; i < 41; i++) {
> +		sprintf(p2, "f%d", i);
> +		symlink(p1, p2);
> +		p1 = p2;
> +		p2 = fname[i % 2];
> +	}
> +	return 0;
> +}
> +
> +void clean_link(void)
> +{
> +	char fname[16] = {0};
> +	int i;
> +	for (i = 0; i < 41; i++) {
> +		sprintf(fname, "f%d", i);
> +		unlink(fname);
> +	}
> +}
> +
> +void *do_realpath(void *ignore)
> +{
> +	char *p = realpath("f40", NULL);
> +	printf("%p\n", p);
> +	if (p != NULL)
> +		printf("%s\n", p);
> +	return NULL;
> +}
> +
> +/* Set different stack sizes and check whether stack overflow occurs. */
> +int do_test(int size)
> +{
> +	pthread_t tid;
> +	pthread_attr_t thread_attr;
> +	pthread_attr_init(&thread_attr);
> +	pthread_attr_setstacksize(&thread_attr, size);
> +
> +	pthread_create(&tid, &thread_attr, do_realpath, NULL);
> +	pthread_join(tid, NULL);
> +	return 0;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	creat_link();
> +	do_test(8192*1024);
> +	do_test(160*1024);
> +	do_test(40*1024);

It think it would be suffice to just check with a small stacksize that triggers
the failure on current code.

> +	clean_link();
> +	printf("\n");
> +	return 0;
> +}
> +
>
  
Paul Eggert Aug. 7, 2020, 11:56 p.m. UTC | #2
On 8/7/20 3:16 AM, 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

I don't understand why malloc is required here.

Isn't the problem that the buffer is being allocated over and over again? And if 
you fix that bug, the function should allocate only 8 KiB via __alloca on 
GNU/Linux. Something like the attached (untested) patch, say. So why not keep 
using the stack? That would be more efficient than resorting to the heap.
  
Paul Eggert Aug. 8, 2020, midnight UTC | #3
On 8/7/20 12:43 PM, Adhemerval Zanella wrote:
> Paul, it might be something to be fixed on gnulib since I noted that both
> gpl and lgpl code uses malloca (which calls alloca if the header is present).

The Gnulib canonicalize.c is a quite-different implementation, and does not use 
alloca.

There is quite a bit of Gnulib code that does use alloca, but it is supposed to 
do so without allocating and using more than 4032 bytes at a time, which is safe 
on all the Gnulib targets we know about.
  
Xiaoming Ni Aug. 8, 2020, 8:54 a.m. UTC | #4
On 2020/8/8 7:56, Paul Eggert wrote:
> I don't understand why malloc is required here.
> 
> Isn't the problem that the buffer is being allocated over and over 
> again? And if you fix that bug, the function should allocate only 8 KiB 
> via __alloca on GNU/Linux. Something like the attached (untested) patch, 
> say. So why not keep using the stack? That would be more efficient than 
> resorting to the heap.
To avoid possible stack overflow risks (the remaining stack space is 
uncertain when realpath is called), should we sacrifice some efficiency 
and reduce the stack space usage? Use malloc instead of alloca (4k+4k).

thanks
  
Xiaoming Ni Aug. 8, 2020, 9:14 a.m. UTC | #5
On 2020/8/8 3:43, Adhemerval Zanella wrote:
> 
> 
> On 07/08/2020 07:16, 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
>>
>> v2: Avoid repeated malloc and free operations. and add testcase
>> v1: https://patches-gcc.linaro.org/patch/39851/
>>
>> Signed-off-by: Xiaoming Ni <nixiaoming@huawei.com>
> 
> Again, we do not use DCO, but rather Copyright assignment.

Do I just need to delete this line: "Signed-off-by: Xiaoming Ni 
<nixiaoming@huawei.com>"?
Does this mean that glibc doesn't use 
Signed-off-by/Reported-by/Tested-by/Reviewed-by?

> Paul, it might be something to be fixed on gnulib since I noted that both
> gpl and lgpl code uses malloca (which calls alloca if the header is present).
> 
> Some comments below regarding testing.
> 
>> ---
>>   stdlib/Makefile       |  3 +-
>>   stdlib/canonicalize.c | 25 +++++++++++++--
>>   stdlib/tst-bz26341.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 98 insertions(+), 3 deletions(-)
>>   create mode 100644 stdlib/tst-bz26341.c
>>
>> diff --git a/stdlib/Makefile b/stdlib/Makefile
>> index 4615f6dfe7..bfdd9036b1 100644
>> --- a/stdlib/Makefile
>> +++ b/stdlib/Makefile
>> @@ -87,7 +87,7 @@ tests		:= tst-strtol tst-strtod testmb testrand testsort testdiv   \
>>   		   tst-makecontext-align test-bz22786 tst-strtod-nan-sign \
>>   		   tst-swapcontext1 tst-setcontext4 tst-setcontext5 \
>>   		   tst-setcontext6 tst-setcontext7 tst-setcontext8 \
>> -		   tst-setcontext9 tst-bz20544
>> +		   tst-setcontext9 tst-bz20544 tst-bz26341
>>   
>>   tests-internal	:= tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
>>   		   tst-tls-atexit tst-tls-atexit-nodelete
> 
> Ok.
> 
>> @@ -98,6 +98,7 @@ ifeq ($(build-hardcoded-path-in-tests),yes)
>>   tests += tst-empty-env
>>   endif
>>   
>> +LDLIBS-tst-bz26341 = -lpthread
> 
> This needs to be $(shared-thread-library).
> 
>>   LDLIBS-test-atexit-race = $(shared-thread-library)
>>   LDLIBS-test-at_quick_exit-race = $(shared-thread-library)
>>   LDLIBS-test-cxa_atexit-race = $(shared-thread-library)
>> diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
>> index cbd885a3c5..c02a8a5800 100644
>> --- a/stdlib/canonicalize.c
>> +++ b/stdlib/canonicalize.c
>> @@ -46,6 +46,7 @@ __realpath (const char *name, char *resolved)
>>     const char *start, *end, *rpath_limit;
>>     long int path_max;
>>     int num_links = 0;
>> +  char *buf = NULL;
>>   
>>     if (name == NULL)
>>       {
> 
> Ok.
> 
>> @@ -163,9 +164,18 @@ __realpath (const char *name, char *resolved)
>>   
>>   	  if (S_ISLNK (st.st_mode))
>>   	    {
>> -	      char *buf = __alloca (path_max);
>>   	      size_t len;
>>   
>> +        if (buf == NULL)
>> +          {
>> +            buf = malloc (path_max);
>> +            if (buf == NULL)
>> +              {
>> +                __set_errno (ENOMEM);
>> +                goto error;
>> +              }
>> +          }
>> +
>>   	      if (++num_links > __eloop_threshold ())
>>   		{
>>   		  __set_errno (ELOOP);
> 
> Ok.
> 
>> @@ -178,7 +188,14 @@ __realpath (const char *name, char *resolved)
>>   	      buf[n] = '\0';
>>   
>>   	      if (!extra_buf)
>> -		extra_buf = __alloca (path_max);
>> +          {
>> +            extra_buf = malloc (path_max);
>> +            if (extra_buf == NULL)
>> +              {
>> +                __set_errno (ENOMEM);
>> +                goto error;
>> +              }
>> +          }
>>   
>>   	      len = strlen (end);
>>   	      if (path_max - n <= len)
> 
> Ok.
> 
>> @@ -210,12 +227,16 @@ __realpath (const char *name, char *resolved)
>>     *dest = '\0';
>>   
>>     assert (resolved == NULL || resolved == rpath);
>> +  free(extra_buf);
>> +  free(buf);
>>     return rpath;
>>   
>>   error:
>>     assert (resolved == NULL || resolved == rpath);
>>     if (resolved == NULL)
>>       free (rpath);
>> +  free (extra_buf);
>> +  free (buf);
>>     return NULL;
>>   }
>>   libc_hidden_def (__realpath)
> 
> Ok.
> 
>> diff --git a/stdlib/tst-bz26341.c b/stdlib/tst-bz26341.c
>> new file mode 100644
>> index 0000000000..0fe095b7d1
>> --- /dev/null
>> +++ b/stdlib/tst-bz26341.c
>> @@ -0,0 +1,73 @@
> 
> This test need the Copyright header and to be properly indented using glibc
> code guideline.
>
Is that it?

/* Copyright (C) 2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.

    The GNU C Library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
    License as published by the Free Software Foundation; either
    version 2.1 of the License, or (at your option) any later version.

    The GNU C Library is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
    Lesser General Public License for more details.

    You should have received a copy of the GNU Lesser General Public
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */


> Use it need to use the libsupport (check support/README-testing.c and other
> tests that use '#include <support/test-driver.c>'.
sorry, I'm not familiar with the glibc test suite

If the test case is successful, 0 is returned. If the test case fails, a 
non-zero value is returned. Is this sufficient?

I see that the main() function is also used in tst-qsort.c.
Is support/test-driver.c mandatory?

thanks
Xiaoming Ni


> 
> 
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <limits.h>
>> +#include <sys/resource.h>
>> +#include <sys/stat.h>
>> +#include <fcntl.h>
>> +#include <string.h>
>> +#include <pthread.h>
>> +
>> +int creat_link(void)
>> +{
>> +	int i;
>> +	int fd;
>> +	char fname[2][16] = {0};
>> +	char *p1 = fname[0];
>> +	char *p2 = fname[1];
>> +	strcpy(p1, "f0");
>> +	fd = open(p1, O_RDONLY|O_CREAT, 0444);
>> +	close(fd);
>> +
>> +	for (i = 0; i < 41; i++) {
>> +		sprintf(p2, "f%d", i);
>> +		symlink(p1, p2);
>> +		p1 = p2;
>> +		p2 = fname[i % 2];
>> +	}
>> +	return 0;
>> +}
>> +
>> +void clean_link(void)
>> +{
>> +	char fname[16] = {0};
>> +	int i;
>> +	for (i = 0; i < 41; i++) {
>> +		sprintf(fname, "f%d", i);
>> +		unlink(fname);
>> +	}
>> +}
>> +
>> +void *do_realpath(void *ignore)
>> +{
>> +	char *p = realpath("f40", NULL);
>> +	printf("%p\n", p);
>> +	if (p != NULL)
>> +		printf("%s\n", p);
>> +	return NULL;
>> +}
>> +
>> +/* Set different stack sizes and check whether stack overflow occurs. */
>> +int do_test(int size)
>> +{
>> +	pthread_t tid;
>> +	pthread_attr_t thread_attr;
>> +	pthread_attr_init(&thread_attr);
>> +	pthread_attr_setstacksize(&thread_attr, size);
>> +
>> +	pthread_create(&tid, &thread_attr, do_realpath, NULL);
>> +	pthread_join(tid, NULL);
>> +	return 0;
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +	creat_link();
>> +	do_test(8192*1024);
>> +	do_test(160*1024);
>> +	do_test(40*1024);
> 
> It think it would be suffice to just check with a small stacksize that triggers
> the failure on current code.
Yes, you're right.

> 
>> +	clean_link();
>> +	printf("\n");
>> +	return 0;
>> +}
>> +
>>
> 
> .
> 

Thanks
Xiaoming Ni
  
Paul Eggert Aug. 9, 2020, 8:44 a.m. UTC | #6
On 8/8/20 1:54 AM, Xiaoming Ni wrote:
> To avoid possible stack overflow risks (the remaining stack space is uncertain 
> when realpath is called), should we sacrifice some efficiency and reduce the 
> stack space usage? Use malloc instead of alloca (4k+4k).

8 KiB of stack in a non-recursive I/O routine is not that big a deal. I don't 
see why we would need to worry about that.
  
Florian Weimer Aug. 9, 2020, 12:38 p.m. UTC | #7
* Paul Eggert:

> On 8/8/20 1:54 AM, Xiaoming Ni wrote:
>> To avoid possible stack overflow risks (the remaining stack space is uncertain 
>> when realpath is called), should we sacrifice some efficiency and reduce the 
>> stack space usage? Use malloc instead of alloca (4k+4k).
>
> 8 KiB of stack in a non-recursive I/O routine is not that big a deal. I don't 
> see why we would need to worry about that.

I have seen a report that the temporary buffer in vfprintf on an
unbuffered stream causes crashes because after a hardware upgrade, the
available stack space was insufficient.  That on-stack buffer is 8 KiB
as well.
  
Paul Eggert Aug. 9, 2020, 5:22 p.m. UTC | #8
On 8/9/20 5:38 AM, Florian Weimer wrote:
> I have seen a report that the temporary buffer in vfprintf on an
> unbuffered stream causes crashes because after a hardware upgrade, the
> available stack space was insufficient.  That on-stack buffer is 8 KiB
> as well.

I have no doubt that there are more bug reports about stack overflows "caused" 
by vfprintf's 8 KiB stack than about those "caused" by getchar's 2 KiB stack. 
But this doesn't mean we should worry overmuch about these small stack 
allocations. It's a fact of life that library routines use a small amount of 
stack space, and on today's processors 8 KiB in a leaf I/O function counts as 
"small" even in multithreaded apps. We shouldn't waste valuable development time 
(or user CPU time) trying to shrink such a function's stack space further.

It's easy to shrink 164 KiB (as in the original bug report) down to 8 KiB, so 
let's do that. The need to shrink stack further does not outweigh the need to 
avoid pressuring the memory allocator and worrying about leaks, so let's quit 
while we're ahead.

If (despite my advice) there is a push to shrink the stack space below 8 KiB, at 
the very least we should introduce no new call to malloc when all processing can 
be done in a single buffer containing only PATH_MAX bytes (which describes the 
overwhelming majority of real-world cases).
  
Adhemerval Zanella Aug. 10, 2020, 1:40 p.m. UTC | #9
On 09/08/2020 14:22, Paul Eggert wrote:
> On 8/9/20 5:38 AM, Florian Weimer wrote:
>> I have seen a report that the temporary buffer in vfprintf on an
>> unbuffered stream causes crashes because after a hardware upgrade, the
>> available stack space was insufficient.  That on-stack buffer is 8 KiB
>> as well.
> 
> I have no doubt that there are more bug reports about stack overflows "caused" by vfprintf's 8 KiB stack than about those "caused" by getchar's 2 KiB stack. But this doesn't mean we should worry overmuch about these small stack allocations. It's a fact of life that library routines use a small amount of stack space, and on today's processors 8 KiB in a leaf I/O function counts as "small" even in multithreaded apps. We shouldn't waste valuable development time (or user CPU time) trying to shrink such a function's stack space further.
> 
> It's easy to shrink 164 KiB (as in the original bug report) down to 8 KiB, so let's do that. The need to shrink stack further does not outweigh the need to avoid pressuring the memory allocator and worrying about leaks, so let's quit while we're ahead.
> 
> If (despite my advice) there is a push to shrink the stack space below 8 KiB, at the very least we should introduce no new call to malloc when all processing can be done in a single buffer containing only PATH_MAX bytes (which describes the overwhelming majority of real-world cases).

Could we remove alloca as well? We can make 4k stack usage on most cases use the
extra 4k only where link are seeing the path.
  
Paul Eggert Aug. 11, 2020, 9:54 a.m. UTC | #10
On 8/10/20 6:40 AM, Adhemerval Zanella wrote:
> Could we remove alloca as well? We can make 4k stack usage on most cases use the
> extra 4k only where link are seeing the path.

Yes, the only "cost" of that is that the function will always use 4K (or 
whatever) instead of typically using far less than 4K.
  

Patch

diff --git a/stdlib/Makefile b/stdlib/Makefile
index 4615f6dfe7..bfdd9036b1 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -87,7 +87,7 @@  tests		:= tst-strtol tst-strtod testmb testrand testsort testdiv   \
 		   tst-makecontext-align test-bz22786 tst-strtod-nan-sign \
 		   tst-swapcontext1 tst-setcontext4 tst-setcontext5 \
 		   tst-setcontext6 tst-setcontext7 tst-setcontext8 \
-		   tst-setcontext9 tst-bz20544
+		   tst-setcontext9 tst-bz20544 tst-bz26341
 
 tests-internal	:= tst-strtod1i tst-strtod3 tst-strtod4 tst-strtod5i \
 		   tst-tls-atexit tst-tls-atexit-nodelete
@@ -98,6 +98,7 @@  ifeq ($(build-hardcoded-path-in-tests),yes)
 tests += tst-empty-env
 endif
 
+LDLIBS-tst-bz26341 = -lpthread
 LDLIBS-test-atexit-race = $(shared-thread-library)
 LDLIBS-test-at_quick_exit-race = $(shared-thread-library)
 LDLIBS-test-cxa_atexit-race = $(shared-thread-library)
diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index cbd885a3c5..c02a8a5800 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -46,6 +46,7 @@  __realpath (const char *name, char *resolved)
   const char *start, *end, *rpath_limit;
   long int path_max;
   int num_links = 0;
+  char *buf = NULL;
 
   if (name == NULL)
     {
@@ -163,9 +164,18 @@  __realpath (const char *name, char *resolved)
 
 	  if (S_ISLNK (st.st_mode))
 	    {
-	      char *buf = __alloca (path_max);
 	      size_t len;
 
+        if (buf == NULL)
+          {
+            buf = malloc (path_max);
+            if (buf == NULL)
+              {
+                __set_errno (ENOMEM);
+                goto error;
+              }
+          }
+
 	      if (++num_links > __eloop_threshold ())
 		{
 		  __set_errno (ELOOP);
@@ -178,7 +188,14 @@  __realpath (const char *name, char *resolved)
 	      buf[n] = '\0';
 
 	      if (!extra_buf)
-		extra_buf = __alloca (path_max);
+          {
+            extra_buf = malloc (path_max);
+            if (extra_buf == NULL)
+              {
+                __set_errno (ENOMEM);
+                goto error;
+              }
+          }
 
 	      len = strlen (end);
 	      if (path_max - n <= len)
@@ -210,12 +227,16 @@  __realpath (const char *name, char *resolved)
   *dest = '\0';
 
   assert (resolved == NULL || resolved == rpath);
+  free(extra_buf);
+  free(buf);
   return rpath;
 
 error:
   assert (resolved == NULL || resolved == rpath);
   if (resolved == NULL)
     free (rpath);
+  free (extra_buf);
+  free (buf);
   return NULL;
 }
 libc_hidden_def (__realpath)
diff --git a/stdlib/tst-bz26341.c b/stdlib/tst-bz26341.c
new file mode 100644
index 0000000000..0fe095b7d1
--- /dev/null
+++ b/stdlib/tst-bz26341.c
@@ -0,0 +1,73 @@ 
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <string.h>
+#include <pthread.h>
+
+int creat_link(void)
+{
+	int i;
+	int fd;
+	char fname[2][16] = {0};
+	char *p1 = fname[0];
+	char *p2 = fname[1];
+	strcpy(p1, "f0");
+	fd = open(p1, O_RDONLY|O_CREAT, 0444);
+	close(fd);
+
+	for (i = 0; i < 41; i++) {
+		sprintf(p2, "f%d", i);
+		symlink(p1, p2);
+		p1 = p2;
+		p2 = fname[i % 2];
+	}
+	return 0;
+}
+
+void clean_link(void)
+{
+	char fname[16] = {0};
+	int i;
+	for (i = 0; i < 41; i++) {
+		sprintf(fname, "f%d", i);
+		unlink(fname);
+	}
+}
+
+void *do_realpath(void *ignore)
+{
+	char *p = realpath("f40", NULL);
+	printf("%p\n", p);
+	if (p != NULL)
+		printf("%s\n", p);
+	return NULL;
+}
+
+/* Set different stack sizes and check whether stack overflow occurs. */
+int do_test(int size)
+{
+	pthread_t tid;
+	pthread_attr_t thread_attr;
+	pthread_attr_init(&thread_attr);
+	pthread_attr_setstacksize(&thread_attr, size);
+
+	pthread_create(&tid, &thread_attr, do_realpath, NULL);
+	pthread_join(tid, NULL);
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	creat_link();
+	do_test(8192*1024);
+	do_test(160*1024);
+	do_test(40*1024);
+	clean_link();
+	printf("\n");
+	return 0;
+}
+