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
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