Re: [PATCH] vsprintf: Fix memory barriers of ptr_key to have_filed_random_ptr_key

From: Steven Rostedt
Date: Tue May 15 2018 - 19:00:32 EST


On Tue, 15 May 2018 18:41:17 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Tue, 15 May 2018 15:31:37 -0700
> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> > > we could always do this:
> >
> > Ugh. I think I prefer the barriers.
>

If it is that if statement you don't like. We can get rid of it. On
early boot, the code is called from preemptable context, it's only
later that it is not. So we know when we can call it directly and when
we need to have a work queue.

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 30c0cb8cc9bc..69d3ed8557ce 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1669,19 +1669,21 @@ char *pointer_string(char *buf, char *end, const void *ptr,
return number(buf, end, (unsigned long int)ptr, spec);
}

-static bool have_filled_random_ptr_key __read_mostly;
+static DEFINE_STATIC_KEY_TRUE(not_filled_random_ptr_key);
static siphash_key_t ptr_key __read_mostly;

-static void fill_random_ptr_key(struct random_ready_callback *unused)
+static void enable_ptr_key_workfn(struct work_struct *work)
{
get_random_bytes(&ptr_key, sizeof(ptr_key));
- /*
- * have_filled_random_ptr_key==true is dependent on get_random_bytes().
- * ptr_to_id() needs to see have_filled_random_ptr_key==true
- * after get_random_bytes() returns.
- */
- smp_mb();
- WRITE_ONCE(have_filled_random_ptr_key, true);
+ /* Needs to run from preemptable context */
+ static_branch_disable(&not_filled_random_ptr_key);
+}
+
+static DECLARE_WORK(enable_ptr_key_work, enable_ptr_key_workfn);
+
+static void fill_random_ptr_key(struct random_ready_callback *unused)
+{
+ queue_work(system_unbound_wq, &enable_ptr_key_work);
}

static struct random_ready_callback random_ready = {
@@ -1695,7 +1697,7 @@ static int __init initialize_ptr_random(void)
if (!ret) {
return 0;
} else if (ret == -EALREADY) {
- fill_random_ptr_key(&random_ready);
+ enable_ptr_key_workfn(&enable_ptr_key_work);
return 0;
}

@@ -1709,7 +1711,7 @@ static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
unsigned long hashval;
const int default_width = 2 * sizeof(ptr);

- if (unlikely(!have_filled_random_ptr_key)) {
+ if (static_branch_unlikely(&not_filled_random_ptr_key)) {
spec.field_width = default_width;
/* string length must be less than default_width */
return string(buf, end, "(ptrval)", spec);

-- Steve