Re: [PATCH] jump label: close race in jump_label_inc() vs. jump_label_dec()

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 2012-01-04 at 10:32 -0500, Jason Baron wrote:
> The previous fix to ensure that jump_label_inc() does not return until the jump
> is completely patched, opened up a race in the inc/dec path. The scenario is:

You forgot something:


> 
> key->enabled = 0;
> 
> 	CPU 0					CPU 1
>         ----- 					-----

  jump_label_lock();

> 
> jump_label_inc():			jump_label_dec():
> 
> 1) if (atomic_read(&key->enabled) == 0)
> 	jump_label_update(key, JUMP_LABEL_ENABLE);
> 
> 					 2) if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex))
> 					 	return;
> 
> 3) atomic_inc(&key->enabled);

  jump_label_unlock();



> 
> So now, key->enabled = 0, but the jump has been enabled, which is an invalid
> state.

How does key->enabled end up == 0?

As Gleb said, it's a higher level bug if we do a jump_label_dec() when
key->enabled already is zero.

Thus, in this scenario, we enter jump_label_inc() with key->enabled=1,
and 1) will not be true. When we hit 2), it will have to grab the
jump_label_mutex, which will be held, thus it will block until CPU 0 is
finished, in which case, key->enabled=1 and the
atomic_dec_and_mutex_lock() will fail and return.

The end result is key->enabled=1 and we have jump labels enabled.

What's the invalid state?

-- Steve