From: Herbert Xu <herbert.xu@redhat.com>
Subject: Re: [RHEL5 PATCH] [BLKTAP]: Fix potential grant entry leaks on error
Date: Wed, 13 Dec 2006 19:02:18 +1100
Bugzilla: 217993
Message-Id: <20061213080218.GA17666@gondor.apana.org.au>
Changelog: Xen: Fix potential grant entry leaks on error


On Tue, Dec 12, 2006 at 07:38:11PM +0000, Don Zickus wrote:
> 
> ACKs?

OK, let's see if we can get some more acks with this version :)

BZ 217993

This patch is from xen-unstable changeset 11835-11837.  It fixes
a security issue in the blktap backend module.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
# HG changeset patch
# User kfraser@localhost.localdomain
# Node ID 1467ae6622288069016fd0a12010fa9e737f6f17
# Parent  4c40bed66adeafc1bea38b7f9620223355b4f44e
[BLKTAP]: Kill duplicate fast_flush_area call

The dispatch_rw_block_io may call fast_flush_area twice if
create_lookup_pte_addr fails (there is a flush call at fail_flush
already).  The second call simply causes warnings to be printed
on the console.  This patch removes the duplicate call.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Index: latest/drivers/xen/blktap/blktapmain.c
===================================================================
--- latest.orig/drivers/xen/blktap/blktapmain.c
+++ latest/drivers/xen/blktap/blktapmain.c
@@ -844,29 +844,30 @@ static void fast_flush_area(pending_req_
 		uvaddr = MMAP_VADDR(info->user_vstart, u_idx, i);
 
 		khandle = &pending_handle(mmap_idx, k_idx, i);
-		if (BLKTAP_INVALID_HANDLE(khandle)) {
-			WPRINTK("BLKTAP_INVALID_HANDLE\n");
-			continue;
-		}
-		gnttab_set_unmap_op(&unmap[invcount], 
+
+		if (khandle->kernel != 0xFFFF) {
+			gnttab_set_unmap_op(&unmap[invcount],
 				    idx_to_kaddr(mmap_idx, k_idx, i), 
 				    GNTMAP_host_map, khandle->kernel);
-		invcount++;
-
-		if (create_lookup_pte_addr(
-		    info->vma->vm_mm,
-		    MMAP_VADDR(info->user_vstart, u_idx, i), 
-		    &ptep) !=0) {
-			WPRINTK("Couldn't get a pte addr!\n");
-			return;
+			invcount++;
 		}
 
-		gnttab_set_unmap_op(&unmap[invcount], ptep,
-			GNTMAP_host_map |
-			GNTMAP_application_map |
-			GNTMAP_contains_pte,
-			khandle->user);
-		invcount++;
+		if (khandle->user != 0xFFFF) {
+			if (create_lookup_pte_addr(
+		    		info->vma->vm_mm,
+		    		MMAP_VADDR(info->user_vstart, u_idx, i),
+		    		&ptep) !=0) 	{
+				WPRINTK("Couldn't get a pte addr!\n");
+				return;
+			}
+
+			gnttab_set_unmap_op(&unmap[invcount], ptep,
+				GNTMAP_host_map |
+				GNTMAP_application_map |
+				GNTMAP_contains_pte,
+				khandle->user);
+			invcount++;
+		}
             
 		BLKTAP_INVALIDATE_HANDLE(khandle);
 	}
@@ -1224,19 +1225,25 @@ static void dispatch_rw_block_io(blkif_t
 		if (unlikely(map[i].status != 0)) {
 			WPRINTK("invalid kernel buffer -- "
 				"could not remap it\n");
-			goto fail_flush;
+			ret |= 1;
+			map[i].handle = 0xFFFF;
 		}
 
 		if (unlikely(map[i+1].status != 0)) {
 			WPRINTK("invalid user buffer -- "
 				"could not remap it\n");
-			goto fail_flush;
+			ret |= 1;
+			map[i+1].handle = 0xFFFF;
 		}
 
 		pending_handle(mmap_idx, pending_idx, i/2).kernel 
 			= map[i].handle;
 		pending_handle(mmap_idx, pending_idx, i/2).user   
 			= map[i+1].handle;
+
+		if (ret)
+			continue;
+
 		set_phys_to_machine(__pa(kvaddr) >> PAGE_SHIFT,
 			FOREIGN_FRAME(map[i].dev_bus_addr >> PAGE_SHIFT));
 		offset = (uvaddr - info->vma->vm_start) >> PAGE_SHIFT;
@@ -1244,6 +1251,10 @@ static void dispatch_rw_block_io(blkif_t
 		((struct page **)info->vma->vm_private_data)[offset] =
 			pg;
 	}
+
+	if (ret)
+		goto fail_flush;
+
 	/* Mark mapped pages as reserved: */
 	for (i = 0; i < req->nr_segments; i++) {
 		unsigned long kvaddr;