[kernel-hardening] [PATCH 0/7] char/random: Simplify random address requests

[kernel-hardening] [PATCH 0/7] char/random: Simplify random address requests

[Jason Cooper]

Two previous attempts have been made to rework this API.  The first can be
found at:

  https://lkml.kernel.org/r/cover.1390770607.git.ydroneaud@opteya.com

The second at:

  https://lkml.kernel.org/r/1469471141-25669-1-git-send-email-william.c.roberts@intel.com

The RFC version of this series can been seen at:

  https://lkml.kernel.org/r/20160726030201.6775-1-jason@lakedaemon.net

In addition to incorporating ideas from these two previous efforts, this series
adds several desirable features.  First, we take the range as an argument
directly, which removes math both before the call and inside the function.
Second, we return the start address on error.  All callers fell back to the
start address on error, so we remove the need to check for errors.  Third, we
cap range to prevent overflow.  Last, we use kerneldoc to describe the new
function.

If possible, I'd like to request Acks from the various subsystems so that we
can merge this as one bisectable branch.

Jason Cooper (7):
  random: Simplify API for random address requests
  x86: Use simpler API for random address requests
  ARM: Use simpler API for random address requests
  arm64: Use simpler API for random address requests
  tile: Use simpler API for random address requests
  unicore32: Use simpler API for random address requests
  random: Remove unused randomize_range()

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

-- 
2.9.2

[kernel-hardening] [PATCH 1/7] random: Simplify API for random address requests

[Jason Cooper]

To date, all callers of randomize_range() have set the length to 0, and
check for a zero return value.  For the current callers, the only way
to get zero returned is if end <= start.  Since they are all adding a
constant to the start address, this is unnecessary.

We can remove a bunch of needless checks by simplifying the API to do
just what everyone wants, return an address between [start, start +
range).

While we're here, s/get_random_int/get_random_long/.  No current call
site is adversely affected by get_random_int(), since all current range
requests are < UINT_MAX.  However, we should match caller expectations
to avoid coming up short (ha!) in the future.

Address generation within [start, start + range) behavior is preserved.

All current callers to randomize_range() chose to use the start address
if randomize_range() failed.  Therefore, we simplify things by just
returning the start address on error.

randomize_range() will be removed once all callers have been converted
over to randomize_addr().

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 drivers/char/random.c  | 26 ++++++++++++++++++++++++++
 include/linux/random.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0158d3bff7e5..3610774bcc53 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1840,6 +1840,32 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
 	return PAGE_ALIGN(get_random_int() % range + start);
 }
 
+/**
+ * randomize_addr - Generate a random, page aligned address
+ * @start:	The smallest acceptable address the caller will take.
+ * @range:	The size of the area, starting at @start, within which the
+ *		random address must fall.
+ *
+ * Before page alignment, the random address generated can be any value from
+ * @start, to @start + @range - 1 inclusive.
+ *
+ * If @start + @range would overflow, @range is capped.
+ *
+ * Return: A page aligned address within [start, start + range).  On error,
+ * @start is returned.
+ */
+unsigned long
+randomize_addr(unsigned long start, unsigned long range)
+{
+	if (range == 0)
+		return start;
+
+	if (start > ULONG_MAX - range)
+		range = ULONG_MAX - start;
+
+	return PAGE_ALIGN(get_random_long() % range + start);
+}
+
 /* Interface for in-kernel drivers of true hardware RNGs.
  * Those devices may produce endless random bits and will be throttled
  * when our pool is full.
diff --git a/include/linux/random.h b/include/linux/random.h
index e47e533742b5..f1ca2fa4c071 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -35,6 +35,7 @@ extern const struct file_operations random_fops, urandom_fops;
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
 unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
+unsigned long randomize_addr(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
 void prandom_bytes(void *buf, size_t nbytes);
-- 
2.9.2

[kernel-hardening] [PATCH 2/7] x86: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 arch/x86/kernel/process.c    | 3 +--
 arch/x86/kernel/sys_x86_64.c | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 96becbbb52e0..a083a2c0744e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -507,8 +507,7 @@ 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_addr(mm->brk, 0x02000000);
 }
 
 /*
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 10e0272d789a..f9cad22808fc 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -101,7 +101,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
@@ -112,9 +111,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);
-			if (new_begin)
-				*begin = new_begin;
+			*begin = randomize_addr(*begin, 0x02000000);
 		}
 	} else {
 		*begin = current->mm->mmap_legacy_base;
-- 
2.9.2

[kernel-hardening] [PATCH 3/7] ARM: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 arch/arm/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 4a803c5a1ff7..02dee671cded 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -314,8 +314,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_addr(mm->brk, 0x02000000);
 }
 
 #ifdef CONFIG_MMU
-- 
2.9.2

[kernel-hardening] [PATCH 4/7] arm64: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 arch/arm64/kernel/process.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6cd2612236dc..11bf454baf86 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -374,12 +374,8 @@ unsigned long arch_align_stack(unsigned long sp)
 
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
-	unsigned long range_end = mm->brk;
-
 	if (is_compat_task())
-		range_end += 0x02000000;
+		return randomize_addr(mm->brk, 0x02000000);
 	else
-		range_end += 0x40000000;
-
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+		return randomize_addr(mm->brk, 0x40000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH 5/7] tile: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 arch/tile/mm/mmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/tile/mm/mmap.c b/arch/tile/mm/mmap.c
index 851a94e6ae58..50f6a693a2b6 100644
--- a/arch/tile/mm/mmap.c
+++ b/arch/tile/mm/mmap.c
@@ -88,6 +88,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_addr(mm->brk, 0x02000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH 6/7] unicore32: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 arch/unicore32/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 00299c927852..b856178cf167 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -295,8 +295,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_addr(mm->brk, 0x02000000);
 }
 
 /*
-- 
2.9.2

[kernel-hardening] [PATCH 7/7] random: Remove unused randomize_range()

[Jason Cooper]

All call sites for randomize_range have been updated to use the much
simpler and more robust randomize_addr.  Remove the now unnecessary
code.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 drivers/char/random.c  | 19 -------------------
 include/linux/random.h |  1 -
 2 files changed, 20 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 3610774bcc53..5333763a4820 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1821,25 +1821,6 @@ unsigned long get_random_long(void)
 }
 EXPORT_SYMBOL(get_random_long);
 
-/*
- * randomize_range() returns a start address such that
- *
- *    [...... <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)
-{
-	unsigned long range = end - len - start;
-
-	if (end <= start + len)
-		return 0;
-	return PAGE_ALIGN(get_random_int() % range + start);
-}
-
 /**
  * randomize_addr - Generate a random, page aligned address
  * @start:	The smallest acceptable address the caller will take.
diff --git a/include/linux/random.h b/include/linux/random.h
index f1ca2fa4c071..1ad877a98186 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -34,7 +34,6 @@ extern const struct file_operations random_fops, urandom_fops;
 
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
-unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
 unsigned long randomize_addr(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.2

[kernel-hardening] Re: [PATCH 1/7] random: Simplify API for random address requests

[Yann Droneaud]

Hi,

Le jeudi 28 juillet 2016 à 20:47 +0000, Jason Cooper a écrit :
> To date, all callers of randomize_range() have set the length to 0,
> and check for a zero return value.  For the current callers, the only
> way to get zero returned is if end <= start.  Since they are all
> adding a constant to the start address, this is unnecessary.
> 
> We can remove a bunch of needless checks by simplifying the API to do
> just what everyone wants, return an address between [start, start +
> range).
> 
> While we're here, s/get_random_int/get_random_long/.  No current call
> site is adversely affected by get_random_int(), since all current
> range requests are < UINT_MAX.  However, we should match caller
> expectations to avoid coming up short (ha!) in the future.
> 
> Address generation within [start, start + range) behavior is
> preserved.
> 
> All current callers to randomize_range() chose to use the start
> address if randomize_range() failed.  Therefore, we simplify things
> by just returning the start address on error.
> 
> randomize_range() will be removed once all callers have been
> converted over to randomize_addr().
> 
> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> ---
>  drivers/char/random.c  | 26 ++++++++++++++++++++++++++
>  include/linux/random.h |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0158d3bff7e5..3610774bcc53 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1840,6 +1840,32 @@ randomize_range(unsigned long start, unsigned
> long end, unsigned long len)
>  	return PAGE_ALIGN(get_random_int() % range + start);
>  }
>  
> +/**
> + * randomize_addr - Generate a random, page aligned address
> + * @start:	The smallest acceptable address the caller will take.
> + * @range:	The size of the area, starting at @start, within which the
> + *		random address must fall.
> + *
> + * Before page alignment, the random address generated can be any value from
> + * @start, to @start + @range - 1 inclusive.
> + *
> + * If @start + @range would overflow, @range is capped.
> + *
> + * Return: A page aligned address within [start, start + range).

PAGE_ALIGN(start + range - 1) can be greater than start + range ..

In the worst case, when start = 0, range = ULONG_MAX, the result would
be 0.

In order to stay in the bounds, the start address must be rounded up,
and the random offset must be rounded down.

Something I haven't found the time to send was looking like this:

  unsigned long base = PAGE_ALIGN(start);

  range -= (base - start);
  range >>= PAGE_SHIFT;

  return base + ((get_random_int() % range) << PAGE_SHIFT);


>   On error,
> + * @start is returned.
> + */
> +unsigned long
> +randomize_addr(unsigned long start, unsigned long range)
> +{
> +	if (range == 0)
> +		return start;
> +
> +	if (start > ULONG_MAX - range)
> +		range = ULONG_MAX - start;
> +
> +	return PAGE_ALIGN(get_random_long() % range + start);
> +}
> +
>  /* Interface for in-kernel drivers of true hardware RNGs.
>   * Those devices may produce endless random bits and will be throttled
>   * when our pool is full.
> diff --git a/include/linux/random.h b/include/linux/random.h
> index e47e533742b5..f1ca2fa4c071 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -35,6 +35,7 @@ extern const struct file_operations random_fops, urandom_fops;
>  unsigned int get_random_int(void);
>  unsigned long get_random_long(void);
>  unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
> +unsigned long randomize_addr(unsigned long start, unsigned long range);
>  
>  u32 prandom_u32(void);
>  void prandom_bytes(void *buf, size_t nbytes);


Regards.

-- 
Yann Droneaud
OPTEYA

[kernel-hardening] Re: [PATCH 4/7] arm64: Use simpler API for random address requests

[Will Deacon]

On Thu, Jul 28, 2016 at 08:47:27PM +0000, Jason Cooper wrote:
> Currently, all callers to randomize_range() set the length to 0 and
> calculate end by adding a constant to the start address.  We can
> simplify the API to remove a bunch of needless checks and variables.
> 
> Use the new randomize_addr(start, range) call to set the requested
> address.
> 
> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> ---
>  arch/arm64/kernel/process.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 6cd2612236dc..11bf454baf86 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -374,12 +374,8 @@ unsigned long arch_align_stack(unsigned long sp)
>  
>  unsigned long arch_randomize_brk(struct mm_struct *mm)
>  {
> -	unsigned long range_end = mm->brk;
> -
>  	if (is_compat_task())
> -		range_end += 0x02000000;
> +		return randomize_addr(mm->brk, 0x02000000);
>  	else
> -		range_end += 0x40000000;
> -
> -	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
> +		return randomize_addr(mm->brk, 0x40000000);

Looks fine to me, once the core code has settled down:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

[kernel-hardening] Re: [PATCH 1/7] random: Simplify API for random address requests

[Jason Cooper]

Hi Yann,

First, thanks for the review!

On Fri, Jul 29, 2016 at 10:59:14AM +0200, Yann Droneaud wrote:
> Le jeudi 28 juillet 2016 à 20:47 +0000, Jason Cooper a écrit :
> > To date, all callers of randomize_range() have set the length to 0,
> > and check for a zero return value.  For the current callers, the only
> > way to get zero returned is if end <= start.  Since they are all
> > adding a constant to the start address, this is unnecessary.
> > 
> > We can remove a bunch of needless checks by simplifying the API to do
> > just what everyone wants, return an address between [start, start +
> > range).
> > 
> > While we're here, s/get_random_int/get_random_long/.  No current call
> > site is adversely affected by get_random_int(), since all current
> > range requests are < UINT_MAX.  However, we should match caller
> > expectations to avoid coming up short (ha!) in the future.
> > 
> > Address generation within [start, start + range) behavior is
> > preserved.
> > 
> > All current callers to randomize_range() chose to use the start
> > address if randomize_range() failed.  Therefore, we simplify things
> > by just returning the start address on error.
> > 
> > randomize_range() will be removed once all callers have been
> > converted over to randomize_addr().
> > 
> > Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> > ---
> >  drivers/char/random.c  | 26 ++++++++++++++++++++++++++
> >  include/linux/random.h |  1 +
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > index 0158d3bff7e5..3610774bcc53 100644
> > --- a/drivers/char/random.c
> > +++ b/drivers/char/random.c
> > @@ -1840,6 +1840,32 @@ randomize_range(unsigned long start, unsigned
> > long end, unsigned long len)
> >  	return PAGE_ALIGN(get_random_int() % range + start);
> >  }
> >  
> > +/**
> > + * randomize_addr - Generate a random, page aligned address
> > + * @start:	The smallest acceptable address the caller will take.
> > + * @range:	The size of the area, starting at @start, within which the
> > + *		random address must fall.
> > + *
> > + * Before page alignment, the random address generated can be any value from
> > + * @start, to @start + @range - 1 inclusive.
> > + *
> > + * If @start + @range would overflow, @range is capped.
> > + *
> > + * Return: A page aligned address within [start, start + range).
> 
> PAGE_ALIGN(start + range - 1) can be greater than start + range ..

Ok, so I need to reword my Return desription. :)

> In the worst case, when start = 0, range = ULONG_MAX, the result would
> be 0.
> 
> In order to stay in the bounds, the start address must be rounded up,
> and the random offset must be rounded down.

Well, I'm trying to preserve existing behavior.  Of which, it seems to
be presumed that start was page aligned.  Since it was used unaltered in
all cases when randomize_range failed.

I'll add that to the kerneldoc.

> Something I haven't found the time to send was looking like this:
> 
>   unsigned long base = PAGE_ALIGN(start);
> 
>   range -= (base - start);

I think the above two lines are unnecessary due to my comment above.

>   range >>= PAGE_SHIFT;
> 
>   return base + ((get_random_int() % range) << PAGE_SHIFT);

However, this is interesting.  Instead of a random address, you're
picking a random page.  If we combine this with the requirement that
start be page aligned, we can remove the PAGE_ALIGN().  Which neatly
handles your first listed concern.

> >   On error,
> > + * @start is returned.
> > + */
> > +unsigned long
> > +randomize_addr(unsigned long start, unsigned long range)
> > +{
> > +	if (range == 0)
> > +		return start;
> > +
> > +	if (start > ULONG_MAX - range)
> > +		range = ULONG_MAX - start;
> > +
> > +	return PAGE_ALIGN(get_random_long() % range + start);

On digging in to this, I found the following scenario:

start=ULONG_MAX, range=ULONG_MAX
	range=0 by our second test
	UB by get_random_long() % 0

This should be mitigated by swapping the tests.  So, we would have:

unsigned long
randomize_addr(unsigned long start, unsigned long range)
{
	if (start > ULONG_MAX - range)
		range = ULONG_MAX - start;

	range >>= PAGE_SHIFT;

	if (range == 0)
		return start;

	return start + ((get_random_long() % range) << PAGE_SHIFT);
}

Look better?

thx,

Jason.

[kernel-hardening] [PATCH v2 0/7] char/random: Simplify random address requests

[Jason Cooper]

Two previous attempts have been made to rework this API.  The first can be
found at:

  https://lkml.kernel.org/r/cover.1390770607.git.ydroneaud@opteya.com

The second at:

  https://lkml.kernel.org/r/1469471141-25669-1-git-send-email-william.c.roberts@intel.com

The RFC version of this series can been seen at:

  https://lkml.kernel.org/r/20160726030201.6775-1-jason@lakedaemon.net

In addition to incorporating ideas from these two previous efforts, this series
adds several desirable features.  First, we take the range as an argument
directly, which removes math both before the call and inside the function.
Second, we return the start address on error.  All callers fell back to the
start address on error, so we remove the need to check for errors.  Third, we
cap range to prevent overflow.  Last, we use kerneldoc to describe the new
function.

If possible, I'd like to request Acks from the various subsystems so that we
can merge this as one bisectable branch.

Changes from v1:
 - Explicitly mention page_aligned start assumption (Yann Droneaud)
 - pick random pages vice random addresses (Yann Droneaud)
 - catch range=0 last
 - Add Ack for arm64 (Will Deacon)

Jason Cooper (7):
  random: Simplify API for random address requests
  x86: Use simpler API for random address requests
  ARM: Use simpler API for random address requests
  arm64: Use simpler API for random address requests
  tile: Use simpler API for random address requests
  unicore32: Use simpler API for random address requests
  random: Remove unused randomize_range()

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

-- 
2.9.2

[kernel-hardening] [PATCH v2 1/7] random: Simplify API for random address requests

[Jason Cooper]

To date, all callers of randomize_range() have set the length to 0, and
check for a zero return value.  For the current callers, the only way
to get zero returned is if end <= start.  Since they are all adding a
constant to the start address, this is unnecessary.

We can remove a bunch of needless checks by simplifying the API to do
just what everyone wants, return an address between [start, start +
range).

While we're here, s/get_random_int/get_random_long/.  No current call
site is adversely affected by get_random_int(), since all current range
requests are < UINT_MAX.  However, we should match caller expectations
to avoid coming up short (ha!) in the future.

All current callers to randomize_range() chose to use the start address
if randomize_range() failed.  Therefore, we simplify things by just
returning the start address on error.

randomize_range() will be removed once all callers have been converted
over to randomize_addr().

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - Explicitly mention page_aligned start assumption (Yann Droneaud)
 - pick random pages vice random addresses (Yann Droneaud)
 - catch range=0 last

 drivers/char/random.c  | 28 ++++++++++++++++++++++++++++
 include/linux/random.h |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0158d3bff7e5..3bedf69546d6 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1840,6 +1840,34 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
 	return PAGE_ALIGN(get_random_int() % range + start);
 }
 
+/**
+ * randomize_addr - Generate a random, page aligned address
+ * @start:	The smallest acceptable address the caller will take.
+ * @range:	The size of the area, starting at @start, within which the
+ *		random address must fall.
+ *
+ * If @start + @range would overflow, @range is capped.
+ *
+ * NOTE: Historical use of randomize_range, which this replaces, presumed that
+ * @start was already page aligned.  This assumption still holds.
+ *
+ * Return: A page aligned address within [start, start + range).  On error,
+ * @start is returned.
+ */
+unsigned long
+randomize_addr(unsigned long start, unsigned long range)
+{
+	if (start > ULONG_MAX - range)
+		range = ULONG_MAX - start;
+
+	range >>= PAGE_SHIFT;
+
+	if (range == 0)
+		return start;
+
+	return start + (get_random_long() % range << PAGE_SHIFT);
+}
+
 /* Interface for in-kernel drivers of true hardware RNGs.
  * Those devices may produce endless random bits and will be throttled
  * when our pool is full.
diff --git a/include/linux/random.h b/include/linux/random.h
index e47e533742b5..f1ca2fa4c071 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -35,6 +35,7 @@ extern const struct file_operations random_fops, urandom_fops;
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
 unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
+unsigned long randomize_addr(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
 void prandom_bytes(void *buf, size_t nbytes);
-- 
2.9.2

[kernel-hardening] [PATCH v2 2/7] x86: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - none

 arch/x86/kernel/process.c    | 3 +--
 arch/x86/kernel/sys_x86_64.c | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 96becbbb52e0..a083a2c0744e 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -507,8 +507,7 @@ 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_addr(mm->brk, 0x02000000);
 }
 
 /*
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 10e0272d789a..f9cad22808fc 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -101,7 +101,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
@@ -112,9 +111,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);
-			if (new_begin)
-				*begin = new_begin;
+			*begin = randomize_addr(*begin, 0x02000000);
 		}
 	} else {
 		*begin = current->mm->mmap_legacy_base;
-- 
2.9.2

[kernel-hardening] [PATCH v2 3/7] ARM: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - none

 arch/arm/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 4a803c5a1ff7..02dee671cded 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -314,8 +314,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_addr(mm->brk, 0x02000000);
 }
 
 #ifdef CONFIG_MMU
-- 
2.9.2

[kernel-hardening] [PATCH v2 4/7] arm64: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
Acked-by: Will Deacon <will.deacon@arm.com>
---
Changes from v1:
 - Add Ack for arm64 (Will Deacon)

 arch/arm64/kernel/process.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6cd2612236dc..11bf454baf86 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -374,12 +374,8 @@ unsigned long arch_align_stack(unsigned long sp)
 
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
-	unsigned long range_end = mm->brk;
-
 	if (is_compat_task())
-		range_end += 0x02000000;
+		return randomize_addr(mm->brk, 0x02000000);
 	else
-		range_end += 0x40000000;
-
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+		return randomize_addr(mm->brk, 0x40000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH v2 5/7] tile: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - none

 arch/tile/mm/mmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/tile/mm/mmap.c b/arch/tile/mm/mmap.c
index 851a94e6ae58..50f6a693a2b6 100644
--- a/arch/tile/mm/mmap.c
+++ b/arch/tile/mm/mmap.c
@@ -88,6 +88,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_addr(mm->brk, 0x02000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH v2 6/7] unicore32: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - none

 arch/unicore32/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 00299c927852..b856178cf167 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -295,8 +295,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_addr(mm->brk, 0x02000000);
 }
 
 /*
-- 
2.9.2

[kernel-hardening] [PATCH v2 7/7] random: Remove unused randomize_range()

[Jason Cooper]

All call sites for randomize_range have been updated to use the much
simpler and more robust randomize_addr.  Remove the now unnecessary
code.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:
 - none

 drivers/char/random.c  | 19 -------------------
 include/linux/random.h |  1 -
 2 files changed, 20 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 3bedf69546d6..f2a11211bfa2 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1821,25 +1821,6 @@ unsigned long get_random_long(void)
 }
 EXPORT_SYMBOL(get_random_long);
 
-/*
- * randomize_range() returns a start address such that
- *
- *    [...... <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)
-{
-	unsigned long range = end - len - start;
-
-	if (end <= start + len)
-		return 0;
-	return PAGE_ALIGN(get_random_int() % range + start);
-}
-
 /**
  * randomize_addr - Generate a random, page aligned address
  * @start:	The smallest acceptable address the caller will take.
diff --git a/include/linux/random.h b/include/linux/random.h
index f1ca2fa4c071..1ad877a98186 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -34,7 +34,6 @@ extern const struct file_operations random_fops, urandom_fops;
 
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
-unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
 unsigned long randomize_addr(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.2

[kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Kees Cook]

On Sat, Jul 30, 2016 at 8:42 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> To date, all callers of randomize_range() have set the length to 0, and
> check for a zero return value.  For the current callers, the only way
> to get zero returned is if end <= start.  Since they are all adding a
> constant to the start address, this is unnecessary.
>
> We can remove a bunch of needless checks by simplifying the API to do
> just what everyone wants, return an address between [start, start +
> range).
>
> While we're here, s/get_random_int/get_random_long/.  No current call
> site is adversely affected by get_random_int(), since all current range
> requests are < UINT_MAX.  However, we should match caller expectations
> to avoid coming up short (ha!) in the future.
>
> All current callers to randomize_range() chose to use the start address
> if randomize_range() failed.  Therefore, we simplify things by just
> returning the start address on error.
>
> randomize_range() will be removed once all callers have been converted
> over to randomize_addr().
>
> Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Changes from v1:
>  - Explicitly mention page_aligned start assumption (Yann Droneaud)
>  - pick random pages vice random addresses (Yann Droneaud)
>  - catch range=0 last
>
>  drivers/char/random.c  | 28 ++++++++++++++++++++++++++++
>  include/linux/random.h |  1 +
>  2 files changed, 29 insertions(+)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0158d3bff7e5..3bedf69546d6 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1840,6 +1840,34 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
>         return PAGE_ALIGN(get_random_int() % range + start);
>  }
>
> +/**
> + * randomize_addr - Generate a random, page aligned address
> + * @start:     The smallest acceptable address the caller will take.
> + * @range:     The size of the area, starting at @start, within which the
> + *             random address must fall.
> + *
> + * If @start + @range would overflow, @range is capped.
> + *
> + * NOTE: Historical use of randomize_range, which this replaces, presumed that
> + * @start was already page aligned.  This assumption still holds.
> + *
> + * Return: A page aligned address within [start, start + range).  On error,
> + * @start is returned.
> + */
> +unsigned long
> +randomize_addr(unsigned long start, unsigned long range)

Since we're changing other things about this, let's try to document
its behavior in its name too and call this "randomize_page" instead.
If it requires a page-aligned value, we should probably also BUG_ON
it, or adjust the start too.

-Kees

> +{
> +       if (start > ULONG_MAX - range)
> +               range = ULONG_MAX - start;
> +
> +       range >>= PAGE_SHIFT;
> +
> +       if (range == 0)
> +               return start;
> +
> +       return start + (get_random_long() % range << PAGE_SHIFT);
> +}
> +
>  /* Interface for in-kernel drivers of true hardware RNGs.
>   * Those devices may produce endless random bits and will be throttled
>   * when our pool is full.
> diff --git a/include/linux/random.h b/include/linux/random.h
> index e47e533742b5..f1ca2fa4c071 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -35,6 +35,7 @@ extern const struct file_operations random_fops, urandom_fops;
>  unsigned int get_random_int(void);
>  unsigned long get_random_long(void);
>  unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
> +unsigned long randomize_addr(unsigned long start, unsigned long range);
>
>  u32 prandom_u32(void);
>  void prandom_bytes(void *buf, size_t nbytes);
> --
> 2.9.2
>



-- 
Kees Cook
Chrome OS & Brillo Security

[kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Jason Cooper]

On Sun, Jul 31, 2016 at 09:46:53AM -0700, Kees Cook wrote:
> On Sat, Jul 30, 2016 at 8:42 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> > To date, all callers of randomize_range() have set the length to 0, and
> > check for a zero return value.  For the current callers, the only way
> > to get zero returned is if end <= start.  Since they are all adding a
> > constant to the start address, this is unnecessary.
> >
> > We can remove a bunch of needless checks by simplifying the API to do
> > just what everyone wants, return an address between [start, start +
> > range).
> >
> > While we're here, s/get_random_int/get_random_long/.  No current call
> > site is adversely affected by get_random_int(), since all current range
> > requests are < UINT_MAX.  However, we should match caller expectations
> > to avoid coming up short (ha!) in the future.
> >
> > All current callers to randomize_range() chose to use the start address
> > if randomize_range() failed.  Therefore, we simplify things by just
> > returning the start address on error.
> >
> > randomize_range() will be removed once all callers have been converted
> > over to randomize_addr().
> >
> > Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> > ---
> > Changes from v1:
> >  - Explicitly mention page_aligned start assumption (Yann Droneaud)
> >  - pick random pages vice random addresses (Yann Droneaud)
> >  - catch range=0 last
> >
> >  drivers/char/random.c  | 28 ++++++++++++++++++++++++++++
> >  include/linux/random.h |  1 +
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > index 0158d3bff7e5..3bedf69546d6 100644
> > --- a/drivers/char/random.c
> > +++ b/drivers/char/random.c
> > @@ -1840,6 +1840,34 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
> >         return PAGE_ALIGN(get_random_int() % range + start);
> >  }
> >
> > +/**
> > + * randomize_addr - Generate a random, page aligned address
> > + * @start:     The smallest acceptable address the caller will take.
> > + * @range:     The size of the area, starting at @start, within which the
> > + *             random address must fall.
> > + *
> > + * If @start + @range would overflow, @range is capped.
> > + *
> > + * NOTE: Historical use of randomize_range, which this replaces, presumed that
> > + * @start was already page aligned.  This assumption still holds.
> > + *
> > + * Return: A page aligned address within [start, start + range).  On error,
> > + * @start is returned.
> > + */
> > +unsigned long
> > +randomize_addr(unsigned long start, unsigned long range)
> 
> Since we're changing other things about this, let's try to document
> its behavior in its name too and call this "randomize_page" instead.

Ack.  Definitely more accurate.

> If it requires a page-aligned value, we should probably also BUG_ON
> it, or adjust the start too.

merf.  So, this whole series started from a suggested cleanup by William
to s/get_random_int/get_random_long/.

The current users have all been stable the way they are for a long time.
Like pre-git long.  So, if this is just a cleanup for those callers, I
don't think we need to do more than we already are.

However, if the intent is for this function to see wider use, then by
all means, we need to handle start != PAGE_ALIGN(start).

Do you have any new call sites in mind?

thx,

Jason.

[kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Kees Cook]

On Sun, Jul 31, 2016 at 1:56 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> On Sun, Jul 31, 2016 at 09:46:53AM -0700, Kees Cook wrote:
>> On Sat, Jul 30, 2016 at 8:42 AM, Jason Cooper <jason@lakedaemon.net> wrote:
>> > To date, all callers of randomize_range() have set the length to 0, and
>> > check for a zero return value.  For the current callers, the only way
>> > to get zero returned is if end <= start.  Since they are all adding a
>> > constant to the start address, this is unnecessary.
>> >
>> > We can remove a bunch of needless checks by simplifying the API to do
>> > just what everyone wants, return an address between [start, start +
>> > range).
>> >
>> > While we're here, s/get_random_int/get_random_long/.  No current call
>> > site is adversely affected by get_random_int(), since all current range
>> > requests are < UINT_MAX.  However, we should match caller expectations
>> > to avoid coming up short (ha!) in the future.
>> >
>> > All current callers to randomize_range() chose to use the start address
>> > if randomize_range() failed.  Therefore, we simplify things by just
>> > returning the start address on error.
>> >
>> > randomize_range() will be removed once all callers have been converted
>> > over to randomize_addr().
>> >
>> > Signed-off-by: Jason Cooper <jason@lakedaemon.net>
>> > ---
>> > Changes from v1:
>> >  - Explicitly mention page_aligned start assumption (Yann Droneaud)
>> >  - pick random pages vice random addresses (Yann Droneaud)
>> >  - catch range=0 last
>> >
>> >  drivers/char/random.c  | 28 ++++++++++++++++++++++++++++
>> >  include/linux/random.h |  1 +
>> >  2 files changed, 29 insertions(+)
>> >
>> > diff --git a/drivers/char/random.c b/drivers/char/random.c
>> > index 0158d3bff7e5..3bedf69546d6 100644
>> > --- a/drivers/char/random.c
>> > +++ b/drivers/char/random.c
>> > @@ -1840,6 +1840,34 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
>> >         return PAGE_ALIGN(get_random_int() % range + start);
>> >  }
>> >
>> > +/**
>> > + * randomize_addr - Generate a random, page aligned address
>> > + * @start:     The smallest acceptable address the caller will take.
>> > + * @range:     The size of the area, starting at @start, within which the
>> > + *             random address must fall.
>> > + *
>> > + * If @start + @range would overflow, @range is capped.
>> > + *
>> > + * NOTE: Historical use of randomize_range, which this replaces, presumed that
>> > + * @start was already page aligned.  This assumption still holds.
>> > + *
>> > + * Return: A page aligned address within [start, start + range).  On error,
>> > + * @start is returned.
>> > + */
>> > +unsigned long
>> > +randomize_addr(unsigned long start, unsigned long range)
>>
>> Since we're changing other things about this, let's try to document
>> its behavior in its name too and call this "randomize_page" instead.
>
> Ack.  Definitely more accurate.
>
>> If it requires a page-aligned value, we should probably also BUG_ON
>> it, or adjust the start too.
>
> merf.  So, this whole series started from a suggested cleanup by William
> to s/get_random_int/get_random_long/.
>
> The current users have all been stable the way they are for a long time.
> Like pre-git long.  So, if this is just a cleanup for those callers, I
> don't think we need to do more than we already are.
>
> However, if the intent is for this function to see wider use, then by
> all means, we need to handle start != PAGE_ALIGN(start).
>
> Do you have any new call sites in mind?

I have no new call sites in mind, but it seems safe to add a BUG_ON to
verify we don't gain callers that don't follow the correct
expectations. (Or maybe WARN and return start.)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Jason Cooper]

Hi Kees,

On Mon, Aug 01, 2016 at 12:47:59PM -0700, Kees Cook wrote:
> On Sun, Jul 31, 2016 at 1:56 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> > On Sun, Jul 31, 2016 at 09:46:53AM -0700, Kees Cook wrote:
> >> On Sat, Jul 30, 2016 at 8:42 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> >> > To date, all callers of randomize_range() have set the length to 0, and
> >> > check for a zero return value.  For the current callers, the only way
> >> > to get zero returned is if end <= start.  Since they are all adding a
> >> > constant to the start address, this is unnecessary.
> >> >
> >> > We can remove a bunch of needless checks by simplifying the API to do
> >> > just what everyone wants, return an address between [start, start +
> >> > range).
> >> >
> >> > While we're here, s/get_random_int/get_random_long/.  No current call
> >> > site is adversely affected by get_random_int(), since all current range
> >> > requests are < UINT_MAX.  However, we should match caller expectations
> >> > to avoid coming up short (ha!) in the future.
> >> >
> >> > All current callers to randomize_range() chose to use the start address
> >> > if randomize_range() failed.  Therefore, we simplify things by just
> >> > returning the start address on error.
> >> >
> >> > randomize_range() will be removed once all callers have been converted
> >> > over to randomize_addr().
> >> >
> >> > Signed-off-by: Jason Cooper <jason@lakedaemon.net>
> >> > ---
> >> > Changes from v1:
> >> >  - Explicitly mention page_aligned start assumption (Yann Droneaud)
> >> >  - pick random pages vice random addresses (Yann Droneaud)
> >> >  - catch range=0 last
> >> >
> >> >  drivers/char/random.c  | 28 ++++++++++++++++++++++++++++
> >> >  include/linux/random.h |  1 +
> >> >  2 files changed, 29 insertions(+)
> >> >
> >> > diff --git a/drivers/char/random.c b/drivers/char/random.c
> >> > index 0158d3bff7e5..3bedf69546d6 100644
> >> > --- a/drivers/char/random.c
> >> > +++ b/drivers/char/random.c
> >> > @@ -1840,6 +1840,34 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
> >> >         return PAGE_ALIGN(get_random_int() % range + start);
> >> >  }
> >> >
> >> > +/**
> >> > + * randomize_addr - Generate a random, page aligned address
> >> > + * @start:     The smallest acceptable address the caller will take.
> >> > + * @range:     The size of the area, starting at @start, within which the
> >> > + *             random address must fall.
> >> > + *
> >> > + * If @start + @range would overflow, @range is capped.
> >> > + *
> >> > + * NOTE: Historical use of randomize_range, which this replaces, presumed that
> >> > + * @start was already page aligned.  This assumption still holds.
> >> > + *
> >> > + * Return: A page aligned address within [start, start + range).  On error,
> >> > + * @start is returned.
> >> > + */
> >> > +unsigned long
> >> > +randomize_addr(unsigned long start, unsigned long range)
> >>
> >> Since we're changing other things about this, let's try to document
> >> its behavior in its name too and call this "randomize_page" instead.
> >
> > Ack.  Definitely more accurate.
> >
> >> If it requires a page-aligned value, we should probably also BUG_ON
> >> it, or adjust the start too.
> >
> > merf.  So, this whole series started from a suggested cleanup by William
> > to s/get_random_int/get_random_long/.
> >
> > The current users have all been stable the way they are for a long time.
> > Like pre-git long.  So, if this is just a cleanup for those callers, I
> > don't think we need to do more than we already are.
> >
> > However, if the intent is for this function to see wider use, then by
> > all means, we need to handle start != PAGE_ALIGN(start).
> >
> > Do you have any new call sites in mind?
> 
> I have no new call sites in mind, but it seems safe to add a BUG_ON to
> verify we don't gain callers that don't follow the correct
> expectations. (Or maybe WARN and return start.)

No, I think BUG_ON is appropriate.  afaict, the only time this will be
encountered is during the development process.

thx,

Jason.

Re: [kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Michael Ellerman]

Jason Cooper <jason@lakedaemon.net> writes:
> On Mon, Aug 01, 2016 at 12:47:59PM -0700, Kees Cook wrote:
>> On Sun, Jul 31, 2016 at 1:56 PM, Jason Cooper <jason@lakedaemon.net> wrote:
>> 
>> I have no new call sites in mind, but it seems safe to add a BUG_ON to
>> verify we don't gain callers that don't follow the correct
>> expectations. (Or maybe WARN and return start.)
>
> No, I think BUG_ON is appropriate.  afaict, the only time this will be
> encountered is during the development process.

Unless it's not.

Why crash someone's system when you could just page align the value
you're given?

cheers

Re: [kernel-hardening] Re: [PATCH v2 1/7] random: Simplify API for random address requests

[Jason Cooper]

On Tue, Aug 02, 2016 at 01:35:13PM +1000, Michael Ellerman wrote:
> Jason Cooper <jason@lakedaemon.net> writes:
> > On Mon, Aug 01, 2016 at 12:47:59PM -0700, Kees Cook wrote:
> >> On Sun, Jul 31, 2016 at 1:56 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> >> 
> >> I have no new call sites in mind, but it seems safe to add a BUG_ON to
> >> verify we don't gain callers that don't follow the correct
> >> expectations. (Or maybe WARN and return start.)
> >
> > No, I think BUG_ON is appropriate.  afaict, the only time this will be
> > encountered is during the development process.
> 
> Unless it's not.
> 
> Why crash someone's system when you could just page align the value
> you're given?

Ack, v3 on it's way.

thx,

Jason.

[kernel-hardening] [PATCH v3 0/7] char/random: Simplify random address requests

[Jason Cooper]

Two previous attempts have been made to rework this API.  The first can be
found at:

  https://lkml.kernel.org/r/cover.1390770607.git.ydroneaud@opteya.com

The second at:

  https://lkml.kernel.org/r/1469471141-25669-1-git-send-email-william.c.roberts@intel.com

Previous versions of this series can been seen at:

RFC:  https://lkml.kernel.org/r/20160726030201.6775-1-jason@lakedaemon.net
 v1:  https://lkml.kernel.org/r/20160728204730.27453-1-jason@lakedaemon.net
 v2:  https://lkml.kernel.org/r/20160730154244.403-1-jason@lakedaemon.net

In addition to incorporating ideas from these two previous efforts, this series
adds several desirable features.  First, we take the range as an argument
directly, which removes math both before the call and inside the function.
Second, we return the start address on error.  All callers fell back to the
start address on error, so we remove the need to check for errors.  Third, we
cap range to prevent overflow.  Last, we use kerneldoc to describe the new
function.

If possible, I'd like to request Acks from the various subsystems so that we
can merge this as one bisectable branch.

Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)
 - PAGE_ALIGN(start) if it wasn't (Kees Cook, Michael Ellerman)

Changes from v1:
 - Explicitly mention page_aligned start assumption (Yann Droneaud)
 - pick random pages vice random addresses (Yann Droneaud)
 - catch range=0 last
 - Add Ack for arm64 (Will Deacon)

Jason Cooper (7):
  random: Simplify API for random address requests
  x86: Use simpler API for random address requests
  ARM: Use simpler API for random address requests
  arm64: Use simpler API for random address requests
  tile: Use simpler API for random address requests
  unicore32: Use simpler API for random address requests
  random: Remove unused randomize_range()

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

-- 
2.9.2

[kernel-hardening] [PATCH v3 1/7] random: Simplify API for random address requests

[Jason Cooper]

To date, all callers of randomize_range() have set the length to 0, and
check for a zero return value.  For the current callers, the only way
to get zero returned is if end <= start.  Since they are all adding a
constant to the start address, this is unnecessary.

We can remove a bunch of needless checks by simplifying the API to do
just what everyone wants, return an address between [start, start +
range).

While we're here, s/get_random_int/get_random_long/.  No current call
site is adversely affected by get_random_int(), since all current range
requests are < UINT_MAX.  However, we should match caller expectations
to avoid coming up short (ha!) in the future.

All current callers to randomize_range() chose to use the start address
if randomize_range() failed.  Therefore, we simplify things by just
returning the start address on error.

randomize_range() will be removed once all callers have been converted
over to randomize_addr().

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)
 - PAGE_ALIGN(start) if it wasn't (Kees Cook, Michael Ellerman)

 drivers/char/random.c  | 33 +++++++++++++++++++++++++++++++++
 include/linux/random.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0158d3bff7e5..61cb434e3bea 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1840,6 +1840,39 @@ randomize_range(unsigned long start, unsigned long end, unsigned long len)
 	return PAGE_ALIGN(get_random_int() % range + start);
 }
 
+/**
+ * randomize_page - Generate a random, page aligned address
+ * @start:	The smallest acceptable address the caller will take.
+ * @range:	The size of the area, starting at @start, within which the
+ *		random address must fall.
+ *
+ * If @start + @range would overflow, @range is capped.
+ *
+ * NOTE: Historical use of randomize_range, which this replaces, presumed that
+ * @start was already page aligned.  We now align it regardless.
+ *
+ * Return: A page aligned address within [start, start + range).  On error,
+ * @start is returned.
+ */
+unsigned long
+randomize_page(unsigned long start, unsigned long range)
+{
+	if (!PAGE_ALIGNED(start)) {
+		range -= PAGE_ALIGN(start) - start;
+		start = PAGE_ALIGN(start);
+	}
+
+	if (start > ULONG_MAX - range)
+		range = ULONG_MAX - start;
+
+	range >>= PAGE_SHIFT;
+
+	if (range == 0)
+		return start;
+
+	return start + (get_random_long() % range << PAGE_SHIFT);
+}
+
 /* Interface for in-kernel drivers of true hardware RNGs.
  * Those devices may produce endless random bits and will be throttled
  * when our pool is full.
diff --git a/include/linux/random.h b/include/linux/random.h
index e47e533742b5..098fec690d65 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -35,6 +35,7 @@ extern const struct file_operations random_fops, urandom_fops;
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
 unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
+unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
 void prandom_bytes(void *buf, size_t nbytes);
-- 
2.9.2

[kernel-hardening] [PATCH v3 2/7] x86: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)

 arch/x86/kernel/process.c    | 3 +--
 arch/x86/kernel/sys_x86_64.c | 5 +----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 96becbbb52e0..8ca7f42d97f3 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -507,8 +507,7 @@ 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_page(mm->brk, 0x02000000);
 }
 
 /*
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index 10e0272d789a..a55ed63b9f91 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -101,7 +101,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
@@ -112,9 +111,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);
-			if (new_begin)
-				*begin = new_begin;
+			*begin = randomize_page(*begin, 0x02000000);
 		}
 	} else {
 		*begin = current->mm->mmap_legacy_base;
-- 
2.9.2

[kernel-hardening] [PATCH v3 3/7] ARM: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)

 arch/arm/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 4a803c5a1ff7..3ee2fb4c9ae6 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -314,8 +314,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_page(mm->brk, 0x02000000);
 }
 
 #ifdef CONFIG_MMU
-- 
2.9.2

[kernel-hardening] [PATCH v3 4/7] arm64: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
Acked-by: Will Deacon <will.deacon@arm.com>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)

 arch/arm64/kernel/process.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6cd2612236dc..6ac2950ffb78 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -374,12 +374,8 @@ unsigned long arch_align_stack(unsigned long sp)
 
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
-	unsigned long range_end = mm->brk;
-
 	if (is_compat_task())
-		range_end += 0x02000000;
+		return randomize_page(mm->brk, 0x02000000);
 	else
-		range_end += 0x40000000;
-
-	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+		return randomize_page(mm->brk, 0x40000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH v3 5/7] tile: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)

 arch/tile/mm/mmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/tile/mm/mmap.c b/arch/tile/mm/mmap.c
index 851a94e6ae58..ef61c597898b 100644
--- a/arch/tile/mm/mmap.c
+++ b/arch/tile/mm/mmap.c
@@ -88,6 +88,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_page(mm->brk, 0x02000000);
 }
-- 
2.9.2

[kernel-hardening] [PATCH v3 6/7] unicore32: Use simpler API for random address requests

[Jason Cooper]

Currently, all callers to randomize_range() set the length to 0 and
calculate end by adding a constant to the start address.  We can
simplify the API to remove a bunch of needless checks and variables.

Use the new randomize_addr(start, range) call to set the requested
address.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v2:
 - s/randomize_addr/randomize_page/ (Kees Cook)

 arch/unicore32/kernel/process.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/unicore32/kernel/process.c b/arch/unicore32/kernel/process.c
index 00299c927852..d7c6b676b3a5 100644
--- a/arch/unicore32/kernel/process.c
+++ b/arch/unicore32/kernel/process.c
@@ -295,8 +295,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_page(mm->brk, 0x02000000);
 }
 
 /*
-- 
2.9.2

[kernel-hardening] [PATCH v3 7/7] random: Remove unused randomize_range()

[Jason Cooper]

All call sites for randomize_range have been updated to use the much
simpler and more robust randomize_addr.  Remove the now unnecessary
code.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 drivers/char/random.c  | 19 -------------------
 include/linux/random.h |  1 -
 2 files changed, 20 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 61cb434e3bea..46d332dd27a4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1821,25 +1821,6 @@ unsigned long get_random_long(void)
 }
 EXPORT_SYMBOL(get_random_long);
 
-/*
- * randomize_range() returns a start address such that
- *
- *    [...... <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)
-{
-	unsigned long range = end - len - start;
-
-	if (end <= start + len)
-		return 0;
-	return PAGE_ALIGN(get_random_int() % range + start);
-}
-
 /**
  * randomize_page - Generate a random, page aligned address
  * @start:	The smallest acceptable address the caller will take.
diff --git a/include/linux/random.h b/include/linux/random.h
index 098fec690d65..9281dbbb7f4a 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -34,7 +34,6 @@ extern const struct file_operations random_fops, urandom_fops;
 
 unsigned int get_random_int(void);
 unsigned long get_random_long(void);
-unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.2

[kernel-hardening] Re: [PATCH v3 7/7] random: Remove unused randomize_range()

[Andrew Morton]

On Wed,  3 Aug 2016 23:39:13 +0000 Jason Cooper <jason@lakedaemon.net> wrote:

> All call sites for randomize_range have been updated to use the much
> simpler and more robust randomize_addr.  Remove the now unnecessary
> code.

"randomize_page'.

I think I'll grab these patches, see if anybody emits any squeaks.

[kernel-hardening] Re: [PATCH v3 7/7] random: Remove unused randomize_range()

[Jason Cooper]

On Wed, Aug 03, 2016 at 04:48:10PM -0700, Andrew Morton wrote:
> On Wed,  3 Aug 2016 23:39:13 +0000 Jason Cooper <jason@lakedaemon.net> wrote:
> 
> > All call sites for randomize_range have been updated to use the much
> > simpler and more robust randomize_addr.  Remove the now unnecessary
> > code.
> 
> "randomize_page'.

Doh!

> I think I'll grab these patches, see if anybody emits any squeaks.

Thanks, Andrew!

thx,

Jason.

[kernel-hardening] Re: [PATCH v3 0/7] char/random: Simplify random address requests

[Kees Cook]

On Wed, Aug 3, 2016 at 4:39 PM, Jason Cooper <jason@lakedaemon.net> wrote:
> Two previous attempts have been made to rework this API.  The first can be
> found at:
>
>   https://lkml.kernel.org/r/cover.1390770607.git.ydroneaud@opteya.com
>
> The second at:
>
>   https://lkml.kernel.org/r/1469471141-25669-1-git-send-email-william.c.roberts@intel.com
>
> Previous versions of this series can been seen at:
>
> RFC:  https://lkml.kernel.org/r/20160726030201.6775-1-jason@lakedaemon.net
>  v1:  https://lkml.kernel.org/r/20160728204730.27453-1-jason@lakedaemon.net
>  v2:  https://lkml.kernel.org/r/20160730154244.403-1-jason@lakedaemon.net
>
> In addition to incorporating ideas from these two previous efforts, this series
> adds several desirable features.  First, we take the range as an argument
> directly, which removes math both before the call and inside the function.
> Second, we return the start address on error.  All callers fell back to the
> start address on error, so we remove the need to check for errors.  Third, we
> cap range to prevent overflow.  Last, we use kerneldoc to describe the new
> function.
>
> If possible, I'd like to request Acks from the various subsystems so that we
> can merge this as one bisectable branch.
>
> Changes from v2:
>  - s/randomize_addr/randomize_page/ (Kees Cook)
>  - PAGE_ALIGN(start) if it wasn't (Kees Cook, Michael Ellerman)
>
> Changes from v1:
>  - Explicitly mention page_aligned start assumption (Yann Droneaud)
>  - pick random pages vice random addresses (Yann Droneaud)
>  - catch range=0 last
>  - Add Ack for arm64 (Will Deacon)
>
> Jason Cooper (7):
>   random: Simplify API for random address requests
>   x86: Use simpler API for random address requests
>   ARM: Use simpler API for random address requests
>   arm64: Use simpler API for random address requests
>   tile: Use simpler API for random address requests
>   unicore32: Use simpler API for random address requests
>   random: Remove unused randomize_range()
>
>  arch/arm/kernel/process.c       |  3 +--
>  arch/arm64/kernel/process.c     |  8 ++------
>  arch/tile/mm/mmap.c             |  3 +--
>  arch/unicore32/kernel/process.c |  3 +--
>  arch/x86/kernel/process.c       |  3 +--
>  arch/x86/kernel/sys_x86_64.c    |  5 +----
>  drivers/char/random.c           | 36 +++++++++++++++++++++++++-----------
>  include/linux/random.h          |  2 +-
>  8 files changed, 33 insertions(+), 30 deletions(-)

This looks great! Thanks for the v3. :)

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Brillo & Chrome OS Security

[kernel-hardening] Re: [PATCH v3 1/7] random: Simplify API for random address requests

[Yann Droneaud]

Hi,

Le mercredi 03 août 2016 à 23:39 +0000, Jason Cooper a écrit :
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0158d3bff7e5..61cb434e3bea 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -1840,6 +1840,39 @@ randomize_range(unsigned long start, unsigned
> long end, unsigned long len)
>  	return PAGE_ALIGN(get_random_int() % range + start);
>  }
>  
> +/**
> + * randomize_page - Generate a random, page aligned address
> + * @start:	The smallest acceptable address the caller will
> take.
> + * @range:	The size of the area, starting at @start, within
> which the
> + *		random address must fall.
> + *
> + * If @start + @range would overflow, @range is capped.
> + *
> + * NOTE: Historical use of randomize_range, which this replaces,
> presumed that
> + * @start was already page aligned.  We now align it regardless.
> + *
> + * Return: A page aligned address within [start, start + range).  On
> error,
> + * @start is returned.
> + */
> +unsigned long
> +randomize_page(unsigned long start, unsigned long range)
> +{

To prevent an underflow if start is not page aligned (but will one
would ever use a non aligned start address *and* range ? ...)

        if (range < PAGE_SIZE)
                return start;


> +	if (!PAGE_ALIGNED(start)) {
> +		range -= PAGE_ALIGN(start) - start;
> +		start = PAGE_ALIGN(start);
> +	}
> +
> +	if (start > ULONG_MAX - range)
> +		range = ULONG_MAX - start;
> +
> +	range >>= PAGE_SHIFT;
> +
> +	if (range == 0)
> +		return start;
> +
> +	return start + (get_random_long() % range << PAGE_SHIFT);
> +}
> +
>  /* Interface for in-kernel drivers of true hardware RNGs.
>   * Those devices may produce endless random bits and will be
> throttled
>   * when our pool is full.
> 

Regards.

-- 
Yann Droneaud
OPTEYA