Re: [PATCH] [nosys] ARM heap overflow check in libnosys

[Richard Earnshaw <rearnsha at arm dot com>]

On 27/02/14 04:43, Joey Ye wrote:
> Original libnosys doesn't check heap overflow. This patch adds an ARM target
> specific check there, without breaking other targets.
> 
> OK to upstream?
> 
> 
> ChangeLog (libgloss):
> 
> 2014-02-27  Joey Ye  <joey.ye@arm.com>
>         libnosys/sbrk.c (errno.h): New include.
>         (errno): Redefine.
>         (STACK_PTR): Define macro for __arm__.
>         (_sbrk): Check STACK_PTR.
> 

I can't approve or reject this, since it's a change to generic code.
However, some comments:

1) You assume that the stack grows downwards and the heap upwards.  On
some machines the stack can grow upwards.  I'm not sure if machines like
this are interesting in this case.
2) You assume that there's a single stack and that the stack is not
allocated within the heap (threaded code often does this)
3) It would be nice if the machine-specific definition could be moved
out into a machine-specific header.
4) I don't think it's safe to undef errno like that.

R.

> diff --git a/libgloss/libnosys/sbrk.c b/libgloss/libnosys/sbrk.c
> index 86c130a..910db20 100644
> --- a/libgloss/libnosys/sbrk.c
> +++ b/libgloss/libnosys/sbrk.c
> @@ -2,6 +2,16 @@
>  
>  #include "config.h"
>  #include <_syslist.h>
> +#include <errno.h>
> +#undef errno
> +extern int errno;
> +
> +#undef STACK_PTR
> +#ifdef __arm__
> +/* Register name faking - works in collusion with the linker.  */
> +register char *stack_ptr asm ("sp");
> +#define STACK_PTR stack_ptr
> +#endif
>  
>  void *
>  _sbrk (incr)
> @@ -15,6 +25,15 @@ _sbrk (incr)
>       heap_end = & end;
>  
>     prev_heap_end = heap_end;
> +
> +#ifdef STACK_PTR
> +  if (heap_end + incr > STACK_PTR)
> +    {
> +      errno = ENOMEM;
> +      return (void *) -1;
> +    }
> +#endif
> +
>     heap_end += incr;
>  
>     return (void *) prev_heap_end;=
> 
>