[PATCH] Fix stack memory protection on targets where the stack grows upward

[PATCH] Fix stack memory protection on targets where the stack grows upward

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

The tst-cputimer1 test fails on hppa.  Using strace to look at the system calls generated by the test,
I observed that there is a mprotect call that passes a non page-aligned addr argument and it fails
with the error EINVAL.

The attached change aligns the old and new guard addresses to page boundaries and fixes the
failing mprotect call.

A version of this patch, hppa/local-stack-grows-up.diff, has been installed in Debian for a long time.
However, the old and new guard values were reversed in the compare.  As a result, the mprotect call
was skipped.  This versions checks that the new_guard value is greater than the old_guard value.

Please install.

Dave
--
John David Anglin	dave.anglin@bell.net



[-- Attachment #2: allocatestack.d.txt --]
[-- Type: text/plain, Size: 1011 bytes --]

2017-04-16  John David Anglin  <danglin@gcc.gnu.org>

	* nptl/allocatestack.c (allocate_stack): Align old and new guard
	addresses to page boundaries when the stack grows up.

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index e5c5f79a82..b3e0e9959f 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -647,8 +647,12 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 			prot) != 0)
 	    goto mprot_error;
 #elif _STACK_GROWS_UP
-	  if (mprotect ((char *) pd - pd->guardsize,
-			pd->guardsize - guardsize, prot) != 0)
+	  char *new_guard = (char *)(((uintptr_t) pd - guardsize) & ~pagesize_m1);
+	  char *old_guard = (char *)(((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);
+	  /* The guard size difference might be > 0, but once rounded
+	     to the nearest page the size difference might be zero.  */
+	  if (new_guard > old_guard
+	      && mprotect (old_guard, new_guard - old_guard, prot) != 0)
 	    goto mprot_error;
 #endif
 

Re: [PATCH] Fix stack memory protection on targets where the stack grows upward

[Andreas Schwab]

On Apr 16 2017, John David Anglin <dave.anglin@bell.net> wrote:

> +	  char *new_guard = (char *)(((uintptr_t) pd - guardsize) & ~pagesize_m1);
> +	  char *old_guard = (char *)(((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);

The lines are too long.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] Fix stack memory protection on targets where the stack grows upward

[Florian Weimer]

* John David Anglin:

>  #elif _STACK_GROWS_UP
> -	  if (mprotect ((char *) pd - pd->guardsize,
> -			pd->guardsize - guardsize, prot) != 0)
> +	  char *new_guard = (char *)(((uintptr_t) pd - guardsize) & ~pagesize_m1);
> +	  char *old_guard = (char *)(((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);
> +	  /* The guard size difference might be > 0, but once rounded
> +	     to the nearest page the size difference might be zero.  */
> +	  if (new_guard > old_guard
> +	      && mprotect (old_guard, new_guard - old_guard, prot) != 0)
>  	    goto mprot_error;
>  #endif
>  

This is essentially HPPA-specific code, right?  So I guess we can do
whatever it takes there.

Re: [PATCH] Fix stack memory protection on targets where the stack grows upward

[John David Anglin]

On 2017-04-16, at 5:27 PM, Florian Weimer wrote:

> This is essentially HPPA-specific code, right?  So I guess we can do
> whatever it takes there.

I believe the metag architecture also has a stack that grows up.

--
John David Anglin	dave.anglin@bell.net

Re: [PATCH] Fix stack memory protection on targets where the stack grows upward

[Florian Weimer]

* John David Anglin:

> On 2017-04-16, at 5:27 PM, Florian Weimer wrote:
>
>> This is essentially HPPA-specific code, right?  So I guess we can do
>> whatever it takes there.
>
> I believe the metag architecture also has a stack that grows up.

I'm sure there are others, but HPPA is the only one with a glibc port.

Re: [PATCH] Fix stack memory protection on targets where the stack grows upward

[John David Anglin]

On 2017-04-17, at 2:57 AM, Florian Weimer wrote:

> * John David Anglin:
> 
>> On 2017-04-16, at 5:27 PM, Florian Weimer wrote:
>> 
>>> This is essentially HPPA-specific code, right?  So I guess we can do
>>> whatever it takes there.
>> 
>> I believe the metag architecture also has a stack that grows up.
> 
> I'm sure there are others, but HPPA is the only one with a glibc port.

Correct.

--
John David Anglin	dave.anglin@bell.net

[PATCH v2] Fix stack memory protection on targets where the stack grows upward

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 444 bytes --]

On 2017-04-16, at 4:06 PM, Andreas Schwab wrote:

> On Apr 16 2017, John David Anglin <dave.anglin@bell.net> wrote:
> 
>> +	  char *new_guard = (char *)(((uintptr_t) pd - guardsize) & ~pagesize_m1);
>> +	  char *old_guard = (char *)(((uintptr_t) pd - pd->guardsize) & ~pagesize_m1);
> 
> The lines are too long.

The long lines are fixed in this version.  Otherwise, unchanged.

Dave
--
John David Anglin	dave.anglin@bell.net



[-- Attachment #2: allocatestack-v2.d.txt --]
[-- Type: text/plain, Size: 1047 bytes --]

2017-04-17  John David Anglin  <danglin@gcc.gnu.org>

	* nptl/allocatestack.c (allocate_stack): Align old and new guard
	addresses to page boundaries when the stack grows up.

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index e5c5f79a82..595a858861 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -647,8 +647,14 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 			prot) != 0)
 	    goto mprot_error;
 #elif _STACK_GROWS_UP
-	  if (mprotect ((char *) pd - pd->guardsize,
-			pd->guardsize - guardsize, prot) != 0)
+	  char *new_guard = (char *)(((uintptr_t) pd - guardsize)
+			             & ~pagesize_m1);
+	  char *old_guard = (char *)(((uintptr_t) pd - pd->guardsize)
+			             & ~pagesize_m1);
+	  /* The guard size difference might be > 0, but once rounded
+	     to the nearest page the size difference might be zero.  */
+	  if (new_guard > old_guard
+	      && mprotect (old_guard, new_guard - old_guard, prot) != 0)
 	    goto mprot_error;
 #endif