On Tue, Dec 09, 2014 at 08:59:30AM +0100, David Hildenbrand wrote: > > The compiler is within its rights to optimize the active_writer local > > variable out of existence, thus re-introducing the possible race with > > the writer that can pass a NULL pointer to wake_up_process(). So you > > really need the ACCESS_ONCE() on the read from cpu_hotplug.active_writer. > > Please see http://lwn.net/Articles/508991/ for more information why > > this is absolutely required. > > You're absolutely right, saw your reply on the other patch just after I sent > this version ... > > So if you agree with the change below, I'll send an updated version! > > > > > > + if (unlikely(active_writer)) > > > + wake_up_process(active_writer); > > > cpuhp_lock_release(); > > > return; > > > } > > > @@ -161,15 +167,17 @@ void cpu_hotplug_begin(void) > > > cpuhp_lock_acquire(); > > > for (;;) { > > > mutex_lock(&cpu_hotplug.lock); > > > + __set_current_state(TASK_UNINTERRUPTIBLE); > > > > You lost me on this one. How does this help? > > > > Thanx, Paul > > Imagine e.g. the following (simplified) scenario: > > CPU1 CPU2 > ---------------------------------------------------------------------------- > !mutex_trylock(&cpu_hotplug.lock) | > | cpu_hotplug.puts_pending == 0 > cpu_hotplug.puts_pending++; | > | cpu_hotplug.refcount != 0 > wake_up_process(active_writer) > | __set_current_state(TASK_UNINTERRUPTIBLE); > | schedule(); > | /* will never be woken up */ > > Therefore we have to move the condition check inside the > __set_current_state(TASK_UNINTERRUPTIBLE) -> schedule(); > section to not miss any wake ups when the condition is satisfied. > > So wake_up_process() will either see TASK_RUNNING and do nothing or see > TASK_UNINTERRUPTIBLE and set it to TASK_RUNNING, so schedule() will in > fact be woken up again.
Or the third alternative would be that 'active_writer' which was running on CPU2 already terminated and wake_up_process() has a non-NULL pointer to task_struct which is already dead. Or is there anything that prevents this use-after-free race? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
