randomize_page is an mm function. It is documented like one. It contains
the history of one. It has the naming convention of one. It looks
just like another very similar function in mm, randomize_stack_top().
And it has always been maintained and updated by mm people. There is no
need for it to be in random.c. In the "which shape does not look like
the other ones" test, pointing to randomize_page() is correct.

So move randomize_page() into mm/util.c, right next to the similar
randomize_stack_top() function.

This commit contains no actual code changes.

Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/char/random.c | 32 --------------------------------
 include/linux/mm.h    |  1 +
 mm/util.c             | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 6d8ccb200c5c..5738cab0079e 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -620,38 +620,6 @@ int __cold random_prepare_cpu(unsigned int cpu)
 }
 #endif
 
-/**
- * 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);
-}
-
 
 /**********************************************************************
  *
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9f44254af8ce..b0183450e484 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2677,6 +2677,7 @@ extern int install_special_mapping(struct mm_struct *mm,
 				   unsigned long flags, struct page **pages);
 
 unsigned long randomize_stack_top(unsigned long stack_top);
+unsigned long randomize_page(unsigned long start, unsigned long range);
 
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
diff --git a/mm/util.c b/mm/util.c
index 3492a9e81aa3..ac63e5ca8b21 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -343,6 +343,38 @@ unsigned long randomize_stack_top(unsigned long stack_top)
 #endif
 }
 
+/**
+ * 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);
+}
+
 #ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
 unsigned long arch_randomize_brk(struct mm_struct *mm)
 {
-- 
2.35.1

On Sat, 14 May 2022 14:05:56 +0200 "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> randomize_page is an mm function. It is documented like one. It contains
> the history of one. It has the naming convention of one. It looks
> just like another very similar function in mm, randomize_stack_top().
> And it has always been maintained and updated by mm people. There is no
> need for it to be in random.c. In the "which shape does not look like
> the other ones" test, pointing to randomize_page() is correct.
> 
> So move randomize_page() into mm/util.c, right next to the similar
> randomize_stack_top() function.
> 
> This commit contains no actual code changes.

hm, does it make sense?

Probably randomize_page() (which used to be called randomize_range())
should have been called randomize_address().  Is it an MM function
then?  Not really - it's simply an application of the random number
generator.  So I think it's more a random thing than an MM thing.

> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -343,6 +343,38 @@ unsigned long randomize_stack_top(unsigned long stack_top)
>  #endif
>  }
>  
> +/**
> + * randomize_page - Generate a random, page aligned address

The patch assumes that drivers/char/random.o is always built into
vmlinux, which appears to be the case.  If some space-conscious person
goes and makes random.o build-time optional then they'll need to make
the appropriate adjustments in util.c.  I see no problems with this.

Hi Andrew,

On Mon, May 16, 2022 at 02:28:00PM -0700, Andrew Morton wrote:
> On Sat, 14 May 2022 14:05:56 +0200 "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
> 
> > randomize_page is an mm function. It is documented like one. It contains
> > the history of one. It has the naming convention of one. It looks
> > just like another very similar function in mm, randomize_stack_top().
> > And it has always been maintained and updated by mm people. There is no
> > need for it to be in random.c. In the "which shape does not look like
> > the other ones" test, pointing to randomize_page() is correct.
> > 
> > So move randomize_page() into mm/util.c, right next to the similar
> > randomize_stack_top() function.
> > 
> > This commit contains no actual code changes.
> 
> hm, does it make sense?
> 
> Probably randomize_page() (which used to be called randomize_range())
> should have been called randomize_address().  Is it an MM function
> then?  Not really - it's simply an application of the random number
> generator.  So I think it's more a random thing than an MM thing.

There are many uses of randomness in the Linux kernel. Your use in mm is
not a special snowflake usage. You want good random integers with
various crypto properties? No problem, you got it. But what you do
with those is your own business. (I'm just a random number dealer.) The
particulars of addresses or page aligned addresses or whatever weird
properties you need out of this thing is your own mm puzzle. It has no
business hanging out here. And as evidence of this, randomize_stack_
top() is also in mm/util.c where it belongs and where the various things
it does can be maintained by people who know a thing or two about mm.
Just imagine all the different types of domain-specific objects that we
could randomize according to certain rules, and how insane it would be
if those all wound up in random.c. 

So with all due respect, I must disagree with you.

> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -343,6 +343,38 @@ unsigned long randomize_stack_top(unsigned long stack_top)
> >  #endif
> >  }
> >  
> > +/**
> > + * randomize_page - Generate a random, page aligned address
> 
> The patch assumes that drivers/char/random.o is always built into
> vmlinux, which appears to be the case.  If some space-conscious person
> goes and makes random.o build-time optional then they'll need to make
> the appropriate adjustments in util.c.  I see no problems with this.

random.o has random_init() that is called by main.o, so just for that
reason alone, it's un-=m-able. Plus the zillion call sites everywhere in
the kernel. It'll always be a builtin (and rightly so too, I think).

Jason

>