random: make random_get_entropy() return an unsigned long

Commit Message

Jason A. Donenfeld April 8, 2022, 5:53 p.m. UTC

Some implementations were returning type `unsigned long`, while others
that fell back to get_cycles() were implicitly returning a `cycles_t` or
an untyped constant int literal. That makes for weird and confusing
code, and basically all code in the kernel already handled it like it
was an `unsigned long`. I recently tried to handle it as the largest
type it could be, a `cycles_t`, but doing so doesn't really help with
much. Instead let's just make random_get_entropy() return an unsigned
long all the time. This also matches the commonly used
`arch_get_random_long()` function, so now RDRAND and RDTSC return the
same sized integer, which means one can fallback to the other more
gracefully.

Cc: Dominik Brodowski <[e-mail]>
Cc: Thomas Gleixner <[e-mail]>
Cc: Theodore Ts'o <[e-mail]>
Signed-off-by: Jason A. Donenfeld <[e-mail]>
---
 drivers/char/random.c | 20 +++++++-------------
 include/linux/timex.h |  2 +-
 2 files changed, 8 insertions(+), 14 deletions(-)

Comments

Thomas Gleixner April 10, 2022, 9:45 a.m. UTC | #1

On Fri, Apr 08 2022 at 19:53, Jason A. Donenfeld wrote:
> Some implementations were returning type `unsigned long`, while others
> that fell back to get_cycles() were implicitly returning a `cycles_t` or
> an untyped constant int literal. That makes for weird and confusing
> code, and basically all code in the kernel already handled it like it
> was an `unsigned long`. I recently tried to handle it as the largest
> type it could be, a `cycles_t`, but doing so doesn't really help with
> much. Instead let's just make random_get_entropy() return an unsigned
> long all the time. This also matches the commonly used
> `arch_get_random_long()` function, so now RDRAND and RDTSC return the
> same sized integer, which means one can fallback to the other more
> gracefully.

Acked-by: Thomas Gleixner <[e-mail]>

Patch

diff --git a/drivers/char/random.c b/drivers/char/random.c
index df43c5060f00..6b01b2be9dd4 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1018,7 +1018,7 @@  int __init rand_initialize(void)
  */
 void add_device_randomness(const void *buf, size_t size)
 {
-	cycles_t cycles = random_get_entropy();
+	unsigned long cycles = random_get_entropy();
 	unsigned long flags, now = jiffies;
 
 	if (crng_init == 0 && size)
@@ -1049,8 +1049,7 @@  struct timer_rand_state {
  */
 static void add_timer_randomness(struct timer_rand_state *state, unsigned int num)
 {
-	cycles_t cycles = random_get_entropy();
-	unsigned long flags, now = jiffies;
+	unsigned long cycles = random_get_entropy(), now = jiffies, flags;
 	long delta, delta2, delta3;
 
 	spin_lock_irqsave(&input_pool.lock, flags);
@@ -1339,8 +1338,7 @@  static void mix_interrupt_randomness(struct work_struct *work)
 void add_interrupt_randomness(int irq)
 {
 	enum { MIX_INFLIGHT = 1U << 31 };
-	cycles_t cycles = random_get_entropy();
-	unsigned long now = jiffies;
+	unsigned long cycles = random_get_entropy(), now = jiffies;
 	struct fast_pool *fast_pool = this_cpu_ptr(&irq_randomness);
 	struct pt_regs *regs = get_irq_regs();
 	unsigned int new_count;
@@ -1353,16 +1351,12 @@  void add_interrupt_randomness(int irq)
 	if (cycles == 0)
 		cycles = get_reg(fast_pool, regs);
 
-	if (sizeof(cycles) == 8)
+	if (sizeof(unsigned long) == 8) {
 		irq_data.u64[0] = cycles ^ rol64(now, 32) ^ irq;
-	else {
+		irq_data.u64[1] = regs ? instruction_pointer(regs) : _RET_IP_;
+	} else {
 		irq_data.u32[0] = cycles ^ irq;
 		irq_data.u32[1] = now;
-	}
-
-	if (sizeof(unsigned long) == 8)
-		irq_data.u64[1] = regs ? instruction_pointer(regs) : _RET_IP_;
-	else {
 		irq_data.u32[2] = regs ? instruction_pointer(regs) : _RET_IP_;
 		irq_data.u32[3] = get_reg(fast_pool, regs);
 	}
@@ -1409,7 +1403,7 @@  static void entropy_timer(struct timer_list *t)
 static void try_to_generate_entropy(void)
 {
 	struct {
-		cycles_t cycles;
+		unsigned long cycles;
 		struct timer_list timer;
 	} stack;
 
diff --git a/include/linux/timex.h b/include/linux/timex.h
index 059b18eb1f1f..5745c90c8800 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -75,7 +75,7 @@ 
  * By default we use get_cycles() for this purpose, but individual
  * architectures may override this in their asm/timex.h header file.
  */
-#define random_get_entropy()	get_cycles()
+#define random_get_entropy()	((unsigned long)get_cycles())
 #endif
 
 /*