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/
Home Products Support Community News


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

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
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: Fri, 4 Mar 2011 17:47:08 +0000
Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>, Jeremy Fitzhardinge <Jeremy.Fitzhardinge@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Delivery-date: Fri, 04 Mar 2011 09:47:48 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1299260301-28955-1-git-send-email-ian.campbell@xxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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" +
"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
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


> 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