Re: Is this a racing bug in page_make_sharable()?

[Nai Xia <nai.xia@gmail.com>]

Hi all,

Last time I reported this bug. And I now see some changes in your Xen
git master branch.
However, I think the problem still remains for ref-checking in page
migration to dom_cow.

I think I can construct a bug by interleaving the two code paths:

in guest_remove_page()              |              in page_make_sharable()
------------------------------------------------------------------------------------------------------------------------------
if ( p2m_is_shared(p2mt) )                       .....
...                                              .....
page = mfn_to_page(mfn);                         .....
                                                  .....

                                                  if ( !get_page_and_type(page, d, PGT_shared_page) )    // success

                                                  .........
                                                  if ( page->count_info != (PGC_allocated | (2 + expected_refcnt)) ) // also pass


if ( unlikely(!get_page(page, d)) )

/* go on to remove page */                       /* go on to add page to cow domain */
-------------------------------------------------------------------------------------------------------------------------------------


is there anything that can already prevent such racing or is this really can happen?


Thanks,

Nai Xia


On 2012年01月17日 18:53, Tim Deegan wrote:
> At 22:43 +0800 on 16 Jan (1326753834), Nai Xia wrote:
>> Hi Grzegorz,
>>
>> As I understand, the purpose of the code in page_make_sharable()
>> checking the ref count is to ensure that nobody unexpected is working
>> on the page, and so we can migrate it to dom_cow, right?
>>
>> ====
>>      /* Check if the ref count is 2. The first from PGT_allocated, and
>>       * the second from get_page_and_type at the top of this function */
>>      if(page->count_info != (PGC_allocated | (2 + expected_refcnt)))
>>      {
>>          /* Return type count back to zero */
>>          put_page_and_type(page);
>>          spin_unlock(&d->page_alloc_lock);
>>          return -E2BIG;
>>      }
>> ====
>>
>> However, it seems to me that this ref check and the following page
>> migration is not atomic( although the operations for type_info ref
>> check seems good) i.e. it's possible that it passed this ref
>> check but just before it goes to dom_cow, someone else gets this page?
>
> Yes, there are a number of races around the p2m code; Anders
> Lagar-Cavilla has been working on fixing the locking around p2m lookups
> to take care of this.
>
> Tim.
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel