WARNING - OLD ARCHIVES

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

xen-devel

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

To: keir.fraser@xxxxxxxxxxxx, christopher.clark@xxxxxxxxxxxx
Subject: [Xen-devel] Bug in use of grant tables in blkback.c error path?
From: Harry Butterworth <harry@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Date: Sun, 06 Nov 2005 12:24:54 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Sun, 06 Nov 2005 12:21:02 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>