linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/mm: remove hack in mmap randomize layout
@ 2011-10-17 20:17 Dan McGee
  2011-10-17 21:51 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Dan McGee @ 2011-10-17 20:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras, linux-kernel, Dan McGee

Since commit 8a0a9bd4db63bc45e301, this comment in mmap_rnd() does not
hold true as the value returned by get_random_int() will in fact be
different every single call. Remove the comment and simplify the code
back to its original desired form.

This reverts commit a5adc91a4b44b5d1 which is no longer necessary.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 arch/powerpc/mm/mmap_64.c |   14 +++-----------
 1 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/mm/mmap_64.c b/arch/powerpc/mm/mmap_64.c
index 5a783d8..67a42ed 100644
--- a/arch/powerpc/mm/mmap_64.c
+++ b/arch/powerpc/mm/mmap_64.c
@@ -53,14 +53,6 @@ static inline int mmap_is_legacy(void)
 	return sysctl_legacy_va_layout;
 }
 
-/*
- * Since get_random_int() returns the same value within a 1 jiffy window,
- * we will almost always get the same randomisation for the stack and mmap
- * region. This will mean the relative distance between stack and mmap will
- * be the same.
- *
- * To avoid this we can shift the randomness by 1 bit.
- */
 static unsigned long mmap_rnd(void)
 {
 	unsigned long rnd = 0;
@@ -68,11 +60,11 @@ static unsigned long mmap_rnd(void)
 	if (current->flags & PF_RANDOMIZE) {
 		/* 8MB for 32bit, 1GB for 64bit */
 		if (is_32bit_task())
-			rnd = (long)(get_random_int() % (1<<(22-PAGE_SHIFT)));
+			rnd = (long)(get_random_int() % (1<<(23-PAGE_SHIFT)));
 		else
-			rnd = (long)(get_random_int() % (1<<(29-PAGE_SHIFT)));
+			rnd = (long)(get_random_int() % (1<<(30-PAGE_SHIFT)));
 	}
-	return (rnd << PAGE_SHIFT) * 2;
+	return rnd << PAGE_SHIFT;
 }
 
 static inline unsigned long mmap_base(void)
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/mm: remove hack in mmap randomize layout
  2011-10-17 20:17 [PATCH] powerpc/mm: remove hack in mmap randomize layout Dan McGee
@ 2011-10-17 21:51 ` David Miller
  2011-10-17 22:43   ` Dan McGee
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2011-10-17 21:51 UTC (permalink / raw)
  To: dpmcgee; +Cc: linuxppc-dev, linux-kernel, paulus

From: Dan McGee <dpmcgee@gmail.com>
Date: Mon, 17 Oct 2011 15:17:36 -0500

> Since commit 8a0a9bd4db63bc45e301, this comment in mmap_rnd() does not
> hold true as the value returned by get_random_int() will in fact be
> different every single call. Remove the comment and simplify the code
> back to its original desired form.
> 
> This reverts commit a5adc91a4b44b5d1 which is no longer necessary.
> 
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>

Can you please fix up all the other architectures which use the same
logic, because they have simply copied over what powerpc does?

At a minimum, Sparc has two such locations.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/mm: remove hack in mmap randomize layout
  2011-10-17 21:51 ` David Miller
@ 2011-10-17 22:43   ` Dan McGee
  2011-10-17 22:53     ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Dan McGee @ 2011-10-17 22:43 UTC (permalink / raw)
  To: David Miller; +Cc: linuxppc-dev, linux-kernel, paulus

On Mon, Oct 17, 2011 at 4:51 PM, David Miller <davem@davemloft.net> wrote:
> From: Dan McGee <dpmcgee@gmail.com>
> Date: Mon, 17 Oct 2011 15:17:36 -0500
>
>> Since commit 8a0a9bd4db63bc45e301, this comment in mmap_rnd() does not
>> hold true as the value returned by get_random_int() will in fact be
>> different every single call. Remove the comment and simplify the code
>> back to its original desired form.
>>
>> This reverts commit a5adc91a4b44b5d1 which is no longer necessary.
>>
>> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
>
> Can you please fix up all the other architectures which use the same
> logic, because they have simply copied over what powerpc does?
>
> At a minimum, Sparc has two such locations.
Aha, I wasn't aware this was also being done elsewhere as there was no
comment to tip me off. I found the one in
arch/sparc/kernel/sys_sparc_64.c (mmap_rnd) and have fixed that
locally and will resend, but I'm not seeing get_random_int() in use
anywhere else in that architecture so I'm not quite sure where your
second mentioned location is- or did you just mean the two calls 2
lines apart in mmap_rnd()?

I also did a quick glance over every other usage and didn't seen any
other architectures doing anything funky, even in a slightly different
way.

-Dan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/mm: remove hack in mmap randomize layout
  2011-10-17 22:43   ` Dan McGee
@ 2011-10-17 22:53     ` David Miller
  2011-10-17 23:05       ` Dan McGee
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2011-10-17 22:53 UTC (permalink / raw)
  To: dpmcgee; +Cc: linuxppc-dev, linux-kernel, paulus

From: Dan McGee <dpmcgee@gmail.com>
Date: Mon, 17 Oct 2011 17:43:11 -0500

> Aha, I wasn't aware this was also being done elsewhere as there was no
> comment to tip me off. I found the one in
> arch/sparc/kernel/sys_sparc_64.c (mmap_rnd) and have fixed that
> locally and will resend, but I'm not seeing get_random_int() in use
> anywhere else in that architecture so I'm not quite sure where your
> second mentioned location is- or did you just mean the two calls 2
> lines apart in mmap_rnd()?

My bad, I thought we had implemented address randomization on 32-bit
sparc as well, unfortunately we haven't even though sparc64 has it
for 32-bit compat tasks.

That should be fixed at some point, but is outside of the scope of
what you're doing.

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] powerpc/mm: remove hack in mmap randomize layout
  2011-10-17 22:53     ` David Miller
@ 2011-10-17 23:05       ` Dan McGee
  2011-10-17 23:09         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Dan McGee @ 2011-10-17 23:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus, linux-kernel, David Miller

Since commit 8a0a9bd4db63bc45e301, this comment in mmap_rnd() does not
hold true as the value returned by get_random_int() will in fact be
different every single call. Remove the comment and simplify the code
back to its original desired form.

This reverts commit a5adc91a4b44b5d1 which is no longer necessary and
also fixes the sparc code that copied this same adjustment.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 arch/powerpc/mm/mmap_64.c        |   14 +++-----------
 arch/sparc/kernel/sys_sparc_64.c |    6 +++---
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/mm/mmap_64.c b/arch/powerpc/mm/mmap_64.c
index 5a783d8..67a42ed 100644
--- a/arch/powerpc/mm/mmap_64.c
+++ b/arch/powerpc/mm/mmap_64.c
@@ -53,14 +53,6 @@ static inline int mmap_is_legacy(void)
 	return sysctl_legacy_va_layout;
 }
 
-/*
- * Since get_random_int() returns the same value within a 1 jiffy window,
- * we will almost always get the same randomisation for the stack and mmap
- * region. This will mean the relative distance between stack and mmap will
- * be the same.
- *
- * To avoid this we can shift the randomness by 1 bit.
- */
 static unsigned long mmap_rnd(void)
 {
 	unsigned long rnd = 0;
@@ -68,11 +60,11 @@ static unsigned long mmap_rnd(void)
 	if (current->flags & PF_RANDOMIZE) {
 		/* 8MB for 32bit, 1GB for 64bit */
 		if (is_32bit_task())
-			rnd = (long)(get_random_int() % (1<<(22-PAGE_SHIFT)));
+			rnd = (long)(get_random_int() % (1<<(23-PAGE_SHIFT)));
 		else
-			rnd = (long)(get_random_int() % (1<<(29-PAGE_SHIFT)));
+			rnd = (long)(get_random_int() % (1<<(30-PAGE_SHIFT)));
 	}
-	return (rnd << PAGE_SHIFT) * 2;
+	return rnd << PAGE_SHIFT;
 }
 
 static inline unsigned long mmap_base(void)
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 908b47a..c2c03c8 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -368,11 +368,11 @@ static unsigned long mmap_rnd(void)
 	if (current->flags & PF_RANDOMIZE) {
 		unsigned long val = get_random_int();
 		if (test_thread_flag(TIF_32BIT))
-			rnd = (val % (1UL << (22UL-PAGE_SHIFT)));
+			rnd = (val % (1UL << (23UL-PAGE_SHIFT)));
 		else
-			rnd = (val % (1UL << (29UL-PAGE_SHIFT)));
+			rnd = (val % (1UL << (30UL-PAGE_SHIFT)));
 	}
-	return (rnd << PAGE_SHIFT) * 2;
+	return rnd << PAGE_SHIFT;
 }
 
 void arch_pick_mmap_layout(struct mm_struct *mm)
-- 
1.7.7

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powerpc/mm: remove hack in mmap randomize layout
  2011-10-17 23:05       ` Dan McGee
@ 2011-10-17 23:09         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2011-10-17 23:09 UTC (permalink / raw)
  To: dpmcgee; +Cc: linuxppc-dev, linux-kernel, paulus

From: Dan McGee <dpmcgee@gmail.com>
Date: Mon, 17 Oct 2011 18:05:23 -0500

> Since commit 8a0a9bd4db63bc45e301, this comment in mmap_rnd() does not
> hold true as the value returned by get_random_int() will in fact be
> different every single call. Remove the comment and simplify the code
> back to its original desired form.
> 
> This reverts commit a5adc91a4b44b5d1 which is no longer necessary and
> also fixes the sparc code that copied this same adjustment.
> 
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>

For sparc part:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-10-17 23:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-17 20:17 [PATCH] powerpc/mm: remove hack in mmap randomize layout Dan McGee
2011-10-17 21:51 ` David Miller
2011-10-17 22:43   ` Dan McGee
2011-10-17 22:53     ` David Miller
2011-10-17 23:05       ` Dan McGee
2011-10-17 23:09         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).