From mboxrd@z Thu Jan 1 00:00:00 1970 From: Harry Butterworth Subject: Bug in use of grant tables in blkback.c error path? Date: Sun, 06 Nov 2005 12:24:54 +0000 Message-ID: <1131279894.6956.17.camel@localhost> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: keir.fraser@cl.cam.ac.uk, christopher.clark@cl.cam.ac.uk Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org In dispatch_rw_block_io after a call to HYPERVISOR_grant_table_op, there is the following code which calls fast_flush_area and breaks out of the loop early if one of the handles returned from HYPERVISOR_grant_table_op is negative: for (i = 0; i < nseg; i++) { if (unlikely(map[i].handle < 0)) { DPRINTK("invalid buffer -- could not remap it\n"); fast_flush_area(pending_idx, nseg); goto bad_descriptor; } phys_to_machine_mapping[__pa(MMAP_VADDR( pending_idx, i)) >> PAGE_SHIFT] = FOREIGN_FRAME(map[i].dev_bus_addr >> PAGE_SHIFT); pending_handle(pending_idx, i) = map[i].handle; } The implementation of fast_flush_area uses pending_handle and is called to flush the whole range nseg but the loop above is setting up pending_handle as it goes along so the handles for the pages after the erroneous one are not set up when fast_flush area is called. Here's the bit of fast_flush_area that uses pending_handle: for (i = 0; i < nr_pages; i++) { handle = pending_handle(idx, i); <<<<<<<<<<<<<<<<< if (handle == BLKBACK_INVALID_HANDLE) continue; unmap[i].host_addr = MMAP_VADDR(idx, i); unmap[i].dev_bus_addr = 0; unmap[i].handle = handle; pending_handle(idx, i) = BLKBACK_INVALID_HANDLE; invcount++; } I also checked the implementation of gnttab_map_grant_ref: static long gnttab_map_grant_ref( gnttab_map_grant_ref_t *uop, unsigned int count) { int i; for ( i = 0; i < count; i++ ) (void)__gnttab_map_grant_ref(&uop[i]); return 0; } gnttab_map_grant_ref seems to keep going and attempt to map the remaining pages after an error is encountered. All in all, this looks like a bug to me where failure to map a grant reference in the middle of a set would result in pages kept mapped in the backend when fast_flush_area fails to clean them up. Am I right? Harry. -- Harry Butterworth