WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [memory sharing] bug on get_page_and_type

To: MaoXiaoyun <tinnycloud@xxxxxxxxxxx>
Subject: Re: [Xen-devel] [memory sharing] bug on get_page_and_type
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Fri, 11 Feb 2011 09:32:18 +0000
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, xen devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, "juihaochiang@xxxxxxxxx" <juihaochiang@xxxxxxxxx>
Delivery-date: Fri, 11 Feb 2011 01:32:57 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <BLU157-w29BD737351D22849AB4F5DAEF0@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <BLU157-ds63E3E55F8D444780B613ADAE20@xxxxxxx> <AANLkTika2afgs8hhdMD024PHGzDFsoWcOK7nJ7ePzG=z@xxxxxxxxxxxxxx> <BLU157-w29AAD53D1BF4AF2892205EDAE20@xxxxxxx> <AANLkTim4O6QXdAJedB1KhECpDhrq2w8FTDXn7irQtk2Y@xxxxxxxxxxxxxx> <20110201142816.GE20638@xxxxxxxxxxxxxxxxxxxxxxx> <BLU157-w243AEA6AA94C1835B206F4DAE40@xxxxxxx> <20110202161837.GQ8286@xxxxxxxxxxxxxxxxxxxxxxx> <BLU157-w23DF5D68A8E1E7781B72BADAED0@xxxxxxx> <20110209095720.GC18135@xxxxxxxxxxxxxxxxxxxxxxx> <BLU157-w29BD737351D22849AB4F5DAEF0@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
At 07:04 +0000 on 11 Feb (1297407854), MaoXiaoyun wrote:
> 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.

That's a good idea.  I'd rather not have the name be anything to do with
"sharable", but we could have a function that does a p2m lookup and a
get-page-and-type, all under the p2m lock, and use it instead of the
lookup-check-getref idiom elsewhere.

Then if (as you say) the make-shareable and nominate-page actions were
covered by the same lock (or potentially even just called the same
function themselves) we would eliminate a lot of races. 

That will be too big a patch to take before 4.1.0 but I'd consider it
immediately after the release.

Tim.

>  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@xxxxxxxxxx
> > To: tinnycloud@xxxxxxxxxxx
> > CC: xen-devel@xxxxxxxxxxxxxxxxxxx; George.Dunlap@xxxxxxxxxxxxx; 
> > juihaochiang@xxxxxxxxx
> > 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@xxxxxxxxxx
> > > > To: tinnycloud@xxxxxxxxxxx
> > > > CC: xen-devel@xxxxxxxxxxxxxxxxxxx; George.Dunlap@xxxxxxxxxxxxx; 
> > > > juihaochiang@xxxxxxxxx
> > > > 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@xxxxxxxxxx>
> > Principal Software Engineer, Xen Platform Team
> > Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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