From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH 2/2] evtchn/fifo: don't corrupt queues if an old tail is linked Date: Tue, 07 Jan 2014 15:50:50 +0000 Message-ID: References: <1386683820-9834-3-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386683820-9834-3-git-send-email-david.vrabel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel , xen-devel@lists.xen.org Cc: Jan Beulich List-Id: xen-devel@lists.xenproject.org On 10/12/2013 13:57, "David Vrabel" wrote: > From: David Vrabel > > An event may still be the tail of a queue even if the queue is now > empty (an 'old tail' event). There is logic to handle the case when > this old tail event needs to be added to the now empty queue (by > checking for q->tail == port). > > However, this does not cover all cases. > > 1. An old tail may be re-added simultaneously with another event. > LINKED is set on the old tail, and the other CPU may misinterpret > this as the old tail still being valid and set LINK instead of > HEAD. All events on this queue will then be lost. > > 2. If the old tail event on queue A is moved to a different queue B > (by changing its VCPU or priority), the event may then be linked > onto queue B. When another event is linked onto queue A it will > check the old tail, see that it is linked (but on queue B) and > overwrite the LINK field, corrupting both queues. > > When an event is linked, save the vcpu id and priority of the queue it > is being linked onto. Use this when linking an event to check if it > is an unlinked old tail event. If it is an old tail event, the old > queue is empty and old_q->tail is invalidated to ensure adding another > event to old_q will update HEAD. The tail is invalidated by setting > it to 0 since the event 0 is never linked. > > The old_q->lock is held while setting LINKED to avoid the race with > the test of LINKED in evtchn_fifo_set_link(). > > Since a event channel may move queues after old_q->lock is acquired, > we must check that we have the correct lock and retry if not. Since > changing VCPUs or priority is expected to be rare events that are > serialized in the guest, we try at most 3 times before dropping the > event. This prevents a malicious guest from repeatedly adjusting > priority to prevent another domain from acquiring old_q->lock. > > Signed-off-by: David Vrabel > --- Acked-by: Keir Fraser