lkrg-org / lkrg

Linux Kernel Runtime Guard
https://lkrg.org
Other
404 stars 72 forks source link

Simplify our get_random_long() compatibility wrapper #234

Closed solardiz closed 1 year ago

solardiz commented 1 year ago

Our current get_random_long compatibility wrapper is weird:

#ifdef P_LKRG_CUSTOM_GET_RANDOM_LONG
static DEFINE_PER_CPU(__u32 [MD5_DIGEST_WORDS], p_get_random_int_hash);

static inline unsigned long get_random_long(void) {

   __u32 *p_hash;
   __u32 p_random_int_secret;
   unsigned long p_ret;

   if (arch_get_random_long(&p_ret))
      return p_ret;

   get_random_bytes(&p_random_int_secret, sizeof(p_random_int_secret));
   p_hash = get_cpu_var(p_get_random_int_hash);

   p_hash[0] += current->pid + jiffies + get_cycles();
   md5_transform(p_hash, &p_random_int_secret);
   p_ret = *(unsigned long *)p_hash;
   put_cpu_var(p_get_random_int_hash);

   return p_ret;
}
#endif

Maybe I forget, but I see no good reason for us to do more than a get_random_bytes of the entire unsigned long sized value.

The arch_get_random_long quick path we may keep or drop. It basically uses the RDRAND instruction on x86. Some would say such reliance (rather than using it as an extra input when [re]seeding) on its proper implementation is inappropriate, but then our current uses aren't that security sensitive (we don't derive long-term cryptographic keys). Anyway, this wrapper is only used on very old kernels.

zx2c4 commented 1 year ago

It looks like you just copied and pasted the code from ancient kernels. Yikes. Indeed, just use get_random_bytes(&l, sizeof(l)) and call it a day.

zx2c4 commented 1 year ago

Actually, looking at this, it appears that you're calling get_random_bytes() on the secret (rather than, say, get_random_once() or similar). That means you have a fresh u32 every time anyway? This makes no sense. So if you go with the basic get_random_bytes(&l, sizeof(l)), you'll probably recover performance anyway.

zx2c4 commented 1 year ago

Also RDRAND is mostly not fast.

solardiz commented 1 year ago

Also RDRAND is mostly not fast.

I suppose get_random_bytes on those old kernels is likely slower still?

Anyway, preserving the coding style for now (to be dealt with project-wide separately):

static inline unsigned long get_random_long(void) {
   unsigned long p_ret;
   get_random_bytes(&p_ret, sizeof(p_ret));
   return p_ret;
}