Revamp randomize_range()

PATCH New Yann Droneaud 1 participant 3 messages Jan 26, 2014

Subsystems (4)

Arm (32-bit) x86 arm64 Random number driver

lore

Share & tools

lore

Yann Droneaud

author

2014-01-26 21:20 UTC #0 lore raw

cover Revamp randomize_range()

Hi,

Please find two patches to remove unused/unwanted behavor on randomize_range() function.

The first one remove the last option (len) which is unused in current kernel. The second one rewrite the function to not fail, allowing to remove the error handling code from the calling functions.

I've not added the maintainers of the various arch modified by the patches, wanted first to have your opinion about the changes.

Regards.

Yann Droneaud (2):
random: remove unused len argument in randomize_range() function
random: don't return 0 in randomize_range()

 arch/arm/kernel/process.c       |  2 +-
 arch/arm64/kernel/process.c     |  2 +-
 arch/tile/mm/mmap.c             |  2 +-
 arch/unicore32/kernel/process.c |  2 +-
 arch/x86/kernel/process.c       |  2 +-
 arch/x86/kernel/sys_x86_64.c    |  5 +----
 drivers/char/random.c           | 19 ++++++++-----------
 include/linux/random.h          |  2 +-
 8 files changed, 15 insertions(+), 21 deletions(-)

Signature · 1 line

1.8.5.3

Yann Droneaud

author

2014-01-26 21:25 UTC #1 lore raw

PATCH 1/2 random: remove unused len argument in randomize_range() function

randomize_range() accepts a third parameter as the size of the object to randomly fit in the range [PAGE_ALIGN(start), PAGE_ALIGN(end)]. The functions returns an address in the range [PAGE_ALIGN(start), PAGE_ALIGN(end - len)].

But no kernel code is currently using the third parameter:
it's hardcoded to 0 in each functions calling randomize_range().

So len argument is useless in the current codebase and can be safely
removed: this patch removes the len argument from randomize_range() and fixes code using randomize_range() with len set to 0.

Link http://lkml.kernel.org/r/[email protected] open here

Cc Theodore Ts'o <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
 arch/arm/kernel/process.c       |  2 +-
 arch/arm64/kernel/process.c     |  2 +-
 arch/tile/mm/mmap.c             |  2 +-
 arch/unicore32/kernel/process.c |  2 +-
 arch/x86/kernel/process.c       |  2 +-
 arch/x86/kernel/sys_x86_64.c    |  2 +-
 drivers/char/random.c           | 17 ++++++-----------
 include/linux/random.h          |  2 +-
 8 files changed, 13 insertions(+), 18 deletions(-)

arch/arm/kernel/process.c +1 -1

index 92f7b15dd221..a13d456cc8d1 100644
@@ -428,7 +428,7 @@ unsigned long get_wchan(struct task_struct *p)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+	return randomize_range(mm->brk, range_end) ? : mm->brk;
 }
 
 #ifdef CONFIG_MMU

arch/arm64/kernel/process.c +1 -1

index 1e5a17811648..855c49be0050 100644
@@ -340,7 +340,7 @@ unsigned long arch_align_stack(unsigned long sp)
 static unsigned long randomize_base(unsigned long base)
 {
 	unsigned long range_end = base + (STACK_RND_MASK << PAGE_SHIFT) + 1;
-	return randomize_range(base, range_end, 0) ? : base;
+	return randomize_range(base, range_end) ? : base;
 }
 
 unsigned long arch_randomize_brk(struct mm_struct *mm)

arch/tile/mm/mmap.c +1 -1

index 851a94e6ae58..bc29e8ce0d27 100644
@@ -89,5 +89,5 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+	return randomize_range(mm->brk, range_end) ? : mm->brk;
 }

arch/unicore32/kernel/process.c +1 -1

index 778ebba80827..79a4423c48ba 100644
@@ -302,7 +302,7 @@ unsigned long get_wchan(struct task_struct *p)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+	return randomize_range(mm->brk, range_end) ? : mm->brk;
 }
 
 /*

arch/x86/kernel/process.c +1 -1

index 3fb8d95ab8b5..2db44a7147d1 100644
@@ -466,6 +466,6 @@ unsigned long arch_align_stack(unsigned long sp)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+	return randomize_range(mm->brk, range_end) ? : mm->brk;
 }

arch/x86/kernel/sys_x86_64.c +1 -1

index 30277e27431a..5cd395f21a25 100644
@@ -96,7 +96,7 @@ static void find_start_end(unsigned long flags, unsigned long *begin,
 		*begin = 0x40000000;
 		*end = 0x80000000;
 		if (current->flags & PF_RANDOMIZE) {
-			new_begin = randomize_range(*begin, *begin + 0x02000000, 0);
+			new_begin = randomize_range(*begin, *begin + 0x02000000);
 			if (new_begin)
 				*begin = new_begin;
 		}

drivers/char/random.c +6 ~~-11~~

index d07575c99a5f..115b5a5381fb 100644
@@ -1634,20 +1634,15 @@ unsigned int get_random_int(void)
 EXPORT_SYMBOL(get_random_int);
 
 /*
- * randomize_range() returns a start address such that
+ * randomize_range() returns a page aligned random address
+ * in range [PAGE_ALIGN(start), PAGE_ALIGN(end)].
  *
- *    [...... <range> .....]
- *  start                  end
- *
- * a <range> with size "len" starting at the return value is inside in the
- * area defined by [start, end], but is otherwise randomized.
  */
 unsigned long
-randomize_range(unsigned long start, unsigned long end, unsigned long len)
+randomize_range(unsigned long start, unsigned long end)
 {
-	unsigned long range = end - len - start;
-
-	if (end <= start + len)
+	if (end <= start)
 		return 0;
-	return PAGE_ALIGN(get_random_int() % range + start);
+
+	return PAGE_ALIGN(get_random_int() % (end - start) + start);
 }

include/linux/random.h +1 -1

index 1cfce0e24dbd..9416a0ff129d 100644
@@ -23,7 +23,7 @@ extern const struct file_operations random_fops, urandom_fops;
 #endif
 
 unsigned int get_random_int(void);
-unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
+unsigned long randomize_range(unsigned long start, unsigned long end);
 
 u32 prandom_u32(void);
 void prandom_bytes(void *buf, int nbytes);

Signature · 1 line

1.8.5.3

Yann Droneaud

author

2014-01-26 21:21 UTC #2 lore raw

PATCH 2/2 random: don't return 0 in randomize_range()

In reply to Yann Droneaud

randomize_range() returns 0 when 'end' address is below 'start'
address: it's an error to pass an invalid range to the function.

Code using randomize_range() deals with such error silently and use the start address instead.

This patch makes randomize_range() issue a warning with WARN_ON() when its parameters are invalid and returns the start address, so that code using the function doesn't have to handle an error which is not supposed to happen. The patch also removes the code handling the error in functions using randomize_range().

Link http://lkml.kernel.org/r/[email protected] open here

Cc Theodore Ts'o <[email protected]>

Signed-off-by Yann Droneaud <[email protected]>

---
 arch/arm/kernel/process.c    | 2 +-
 arch/tile/mm/mmap.c          | 2 +-
 arch/x86/kernel/process.c    | 2 +-
 arch/x86/kernel/sys_x86_64.c | 5 +----
 drivers/char/random.c        | 4 +++-
 5 files changed, 7 insertions(+), 8 deletions(-)

arch/arm/kernel/process.c +1 -1

index a13d456cc8d1..005a8ba04739 100644
@@ -428,7 +428,7 @@ unsigned long get_wchan(struct task_struct *p)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end) ? : mm->brk;
+	return randomize_range(mm->brk, range_end);
 }
 
 #ifdef CONFIG_MMU

arch/tile/mm/mmap.c +1 -1

index bc29e8ce0d27..294292714f34 100644
@@ -89,5 +89,5 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end) ? : mm->brk;
+	return randomize_range(mm->brk, range_end);
 }

arch/x86/kernel/process.c +1 -1

index 2db44a7147d1..076358128404 100644
@@ -466,6 +466,6 @@ unsigned long arch_align_stack(unsigned long sp)
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
 	unsigned long range_end = mm->brk + 0x02000000;
-	return randomize_range(mm->brk, range_end) ? : mm->brk;
+	return randomize_range(mm->brk, range_end);
 }

arch/x86/kernel/sys_x86_64.c +1 -4

index 5cd395f21a25..7b2a029e63fb 100644
@@ -85,7 +85,6 @@ static void find_start_end(unsigned long flags, unsigned long *begin,
 			   unsigned long *end)
 {
 	if (!test_thread_flag(TIF_ADDR32) && (flags & MAP_32BIT)) {
-		unsigned long new_begin;
 		/* This is usually used needed to map code in small
 		   model, so it needs to be in the first 31bit. Limit
 		   it to that.  This means we need to move the

@@ -96,9 +95,7 @@ static void find_start_end(unsigned long flags, unsigned long *begin,
 		*begin = 0x40000000;
 		*end = 0x80000000;
 		if (current->flags & PF_RANDOMIZE) {
-			new_begin = randomize_range(*begin, *begin + 0x02000000);
-			if (new_begin)
-				*begin = new_begin;
+			*begin = randomize_range(*begin, *begin + 0x02000000);
 		}
 	} else {
 		*begin = current->mm->mmap_legacy_base;

drivers/char/random.c +3 -1

index 115b5a5381fb..7c7c47e220ca 100644
@@ -1641,8 +1641,10 @@ EXPORT_SYMBOL(get_random_int);
 unsigned long
 randomize_range(unsigned long start, unsigned long end)
 {
+	WARN_ON(end <= start);
+
 	if (end <= start)
-		return 0;
+		return start;
 
 	return PAGE_ALIGN(get_random_int() % (end - start) + start);
 }

Signature · 1 line

1.8.5.3