[Xen-devel] Bug in use of grant tables in blkback.c error path?

[Harry Butterworth <harry@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>]

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 <harry@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>


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