RE: [memory sharing] bug on get_page_and_type

[MaoXiaoyun <tinnycloud@hotmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3644 bytes --]


Thanks Tim.
 
After discuss with JuiHao, How about fix in this way?
 
1)  Suppose we have a function,  make_page_unsharable() to substitude 
p2m_is_shared() check, if p2mt is not shared, we increase its type count 
to avoid it turn to shared while using it.
 
 1 int make_page_unsharable(int enable)
  2 {
  3     p2m_type_t p2mt;
  4     unsigned long mfn;
  5 
  6     p2m_lock()
  7     mfn = mfn_x(gfn_to_mfn(d, gmfn, &p2mt))
  8 
  9     if(p2m_is_shared(p2mt)){
10         p2m_unlock()
11         return 1;
12     }
13                                                                                                                                                          
14     get_page_type() / ***increase page type count to avoid page type turn to shared, since in 
                                                   mem_sharing_nominate_page->page_make_sharable, only type count zero is
                                                    allowed to be shared*/
15     p2m_unlock()
16     
 17     return 0;
18 }   
 
2) If p2mt is not shared, we must decrease it type count after we finish using it
3) To avoid competition, page_make_sharble() and p2m_change_type() in  
mem_sharing_nominate_page() should be protected in same p2m_lock.

comments?
 

> Date: Wed, 9 Feb 2011 09:57:20 +0000
> From: Tim.Deegan@citrix.com
> To: tinnycloud@hotmail.com
> CC: xen-devel@lists.xensource.com; George.Dunlap@eu.citrix.com; juihaochiang@gmail.com
> Subject: Re: [Xen-devel] [memory sharing] bug on get_page_and_type
> 
> At 02:46 +0000 on 09 Feb (1297219562), MaoXiaoyun wrote:
> > I've been looking into the TOCTOU issue quite a while, but
> > 
> > 1) There are quite a lot judgements like "p2m_is_shared(p2mt)" or
> > "p2mt == p2m_ram_shared", which, for me, is hard to tell whom
> > are need to be protect by p2m_lock and whom are not So is
> > there a rule to distinguish these?
> 
> Not particularly. I suspect that most of them will need to be
> changed, but as I said I hope we can find something nicer than
> scattering p2m_lock() around non-p2m code. 
> 
> > 2) Could we improve p2m_lock to sparse lock, which maybe better, right?
> 
> Maybe, but not necessarily. Let's get it working properly first and
> then we can measure lock contention and see whether fancy locks are
> worthwhile. 
> 
> Tim.
> 
> > 
> > > Date: Wed, 2 Feb 2011 16:18:37 +0000
> > > From: Tim.Deegan@citrix.com
> > > To: tinnycloud@hotmail.com
> > > CC: xen-devel@lists.xensource.com; George.Dunlap@eu.citrix.com; juihaochiang@gmail.com
> > > Subject: Re: [Xen-devel] [memory sharing] bug on get_page_and_type
> > >
> > > At 15:43 +0000 on 02 Feb (1296661396), MaoXiaoyun wrote:
> > > > Hi Tim:
> > > >
> > > > Thanks for both your advice and quick reply. I will follow.
> > > >
> > > > So at last we should replace shr_lock with p2m_lock.
> > > > But more complicate, it seems both the
> > > > *check action* code and *nominate page* code need to be locked ,right?
> > > > If so, quite a lot of *check action* codes need to be locked.
> > >
> > > Yes, I think you're right about that. Unfortunately there are some very
> > > long TOCTOU windows in those kind of p2m lookups, which will get more
> > > important as the p2m gets more dynamic. I don't want to have the
> > > callers of p2m code touching the p2m lock directly so we may need a new
> > > p2m interface to address it.
> > >
> > > Tim.
> > >
> 
> -- 
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, Xen Platform Team
> Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
 		 	   		  

[-- Attachment #1.2: Type: text/html, Size: 10122 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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