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

[Xen-devel] Re: [PATCH] xen/p2m/m2p/gnttab: do not add failed grant maps

To: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] xen/p2m/m2p/gnttab: do not add failed grant maps to m2p override
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Wed, 9 Mar 2011 16:54:30 +0000
Cc: Daniel, Graaf <dgdegra@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Jeremy Fitzhardinge <Jeremy.Fitzhardinge@xxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Wed, 09 Mar 2011 08:55:26 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20110309164418.GA8049@xxxxxxxxxxxx>
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>
Organization: Citrix Systems, Inc.
References: <1299260301-28955-1-git-send-email-ian.campbell@xxxxxxxxxx> <1299260828.17339.21.camel@xxxxxxxxxxxxxxxxxxxxxx> <20110309164418.GA8049@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Wed, 2011-03-09 at 16:44 +0000, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 04, 2011 at 05:47:08PM +0000, Ian Campbell wrote:
> > On Fri, 2011-03-04 at 17:38 +0000, Ian Campbell wrote:
> > > The caller will not undo a mapping which failed and therefore the
> > > override will not be removed.
> > > 
> > > This is especially bad in the case of GNTMAP_contains_pte mapping type
> > > mappings where m2p_add_override will destroy the kernel mapping of the
> > > page.
> > > 
> > > This was observed via a failure of map_grant_pages in gntdev_mmap (due
> > > to userspace using a bad grant reference), which left the page in
> > > question unmapped (because it was a GNTMAP_contains_pte mapping) which
> > > led to a crash later on.
> > 
> > My original motivation for looking into this was a dom0 kernel crash in
> > gntdev exposed by using the qemu disk backend on 2.6.32 (by mistake due
> > to not loading blktap). The crash was in mn_release but was root caused
> > by a map_grant_pages() failing (due to a bogus grant ref).
> > 
> > It seems that Daniel's "xen-gntdev: Add reference counting to maps" +
> 
> This one:
> http://article.gmane.org/gmane.comp.emulators.xen.devel/100076 ?

The one with title "Add reference counting to maps", it is
68b025c813c2eb41ff25628e3d4952d5185eb1a4 in your tree.

> > "xen/p2m/m2p/gnttab: do not add failed grant maps to m2p override" +
> > this fix are sufficient to fix it for 2.6.38-rc7+ but I wonder if we
> 
> Can you point me to the "this fix" ?

The fix from the mail I was replying to e.g.
http://lists.xensource.com/archives/html/xen-devel/2011-03/msg00264.html

>  You mention three patches but
> the last one is the same as 
> "xen/p2m/m2p/gnttab: do not add failed grant maps to m2p override"

I think I cut & pasted the wrong subject for one of them. The three
patches are:
        "xen-gntdev: Add reference counting to maps" (68b025c813c2e in your 
tree)
        "xen-gntdev: Fix memory leak when mmap fails" (90b6f30548a52 in your 
tree)
        "xen/p2m/m2p/gnttab: do not add failed grant maps to m2p override" (AKA 
this fix)

I think the first two are sufficient for 2.6.32 since it doesn't have
the m2p override so it doesn't need the third one.

Ian.

> 
> so that looks to be two patches.
> 
> > also want it fixed for 2.6.32? I think in that case backporting Daniel's
> > patches is the best bet. (this fix won't be necessary, no m2p overlay in
> > 2.6.32).
> > 
> > Ian.
> > 
> > > 
> > > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> > > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> > > ---
> > >  drivers/xen/grant-table.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> > > index 1a9bc2b..d0e30ce 100644
> > > --- a/drivers/xen/grant-table.c
> > > +++ b/drivers/xen/grant-table.c
> > > @@ -462,6 +462,10 @@ int gnttab_map_refs(struct gnttab_map_grant_ref 
> > > *map_ops,
> > >           return ret;
> > >  
> > >   for (i = 0; i < count; i++) {
> > > +         /* Do not add to override if the map failed. */
> > > +         if (map_ops[i].status)
> > > +                 continue;
> > > +
> > >           if (map_ops[i].flags & GNTMAP_contains_pte) {
> > >                   pte = (pte_t *) 
> > > (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> > >                           (map_ops[i].host_addr & ~PAGE_MASK));
> > 



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

<Prev in Thread] Current Thread [Next in Thread>