Messages in this thread | | | Date | Tue, 27 Oct 2009 10:00:15 -0400 | From | Gregory Haskins <> | Subject | Re: [KVM PATCH v3 1/3] KVM: fix race in irq_routing logic |
| |
Gregory Haskins wrote: > Gleb Natapov wrote: >> On Mon, Oct 26, 2009 at 12:21:57PM -0400, Gregory Haskins wrote: >>> The current code suffers from the following race condition: >>> >>> thread-1 thread-2 >>> ----------------------------------------------------------- >>> >>> kvm_set_irq() { >>> rcu_read_lock() >>> irq_rt = rcu_dereference(table); >>> rcu_read_unlock(); >>> >>> kvm_set_irq_routing() { >>> mutex_lock(); >>> irq_rt = table; >>> rcu_assign_pointer(); >>> mutex_unlock(); >>> synchronize_rcu(); >>> >>> kfree(irq_rt); >>> >>> irq_rt->entry->set(); /* bad */ >>> >> This is not what happens. irq_rt is never accessed outside read-side >> critical section. > > Sorry, I was generalizing to keep the comments short. I figured it > would be clear what I was actually saying, but realize in retrospect > that I was a little ambiguous.
Here is a revised problem statement
thread-1 thread-2 -----------------------------------------------------------
kvm_set_irq() { rcu_read_lock() irq_rt = rcu_dereference(table); entry_cache = get_entries(irq_rt); rcu_read_unlock();
invalidate_entries(irq_rt);
for_each_entry(entry_cache) entry->set(); /* bad */
-------------------------------------------------------------
"invalidate_entries()" may be any operation that deletes an entry at run-time (doesn't exist today), or as the guest is shutting down. As far as I can tell, the current code does not protect us from either condition, and my proposed patch protects us from both. Did I miss anything?
HTH -Greg
[unhandled content-type:application/pgp-signature] | |