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

Re: [Xen-devel] Re: [PATCH] Fix wild ptr deref during device destruction

On Thu, 2010-02-25 at 04:57 -0500, Daniel Stodden wrote:
> On Thu, 2010-02-25 at 03:28 -0500, Jan Beulich wrote:
> > Wouldn't it be better to move blk_cleanup_queue() even before del_gendisk()?
> 
> No.

Well, I beg you to differ. Maybe this changed, after all this is 2.6.3x.

Cheers,
Daniel

> [2009-09-22 12:48:58 UTC] Call Trace:
> [2009-09-22 12:48:58 UTC]  [<c01d0186>] ? sysfs_remove_dir+0x46/0xa0
> [2009-09-22 12:48:58 UTC]  [<c020180f>] ? kobject_del+0xf/0x30
> [2009-09-22 12:48:58 UTC]  [<c01f107c>] ? __elv_unregister_queue+0x1c/0x20
> [2009-09-22 12:48:58 UTC]  [<c01f108f>] ? elv_unregister_queue+0xf/0x20
> [2009-09-22 12:48:58 UTC]  [<c01f512a>] ? blk_unregister_queue+0x2a/0x70
> [2009-09-22 12:48:58 UTC]  [<c01fa55a>] ? unlink_gendisk+0x2a/0x40
> [2009-09-22 12:48:58 UTC]  [<c01c9b10>] ? del_gendisk+0x60/0xd0
> [2009-09-22 12:48:58 UTC]  [<c028066e>] ? destroy_backdev+0x7e/0x100
> [2009-09-22 12:48:58 UTC]  [<c027f05b>] ? tap_blkif_schedule+0x5cb/0x830
> [2009-09-22 12:48:58 UTC]  [<c011ed51>] ? pick_next_task_fair+0x91/0xd0
> [2009-09-22 12:48:58 UTC]  [<c013dd70>] ? autoremove_wake_function+0x0/0x50
> [2009-09-22 12:48:58 UTC]  [<c027ea90>] ? tap_blkif_schedule+0x0/0x830
> [2009-09-22 12:48:58 UTC]  [<c013da12>] ? kthread+0x42/0x70
> [2009-09-22 12:48:58 UTC]  [<c013d9d0>] ? kthread+0x0/0x70
> [2009-09-22 12:48:58 UTC]  [<c010561b>] ? kernel_thread_helper+0x7/0x10
> 
> changeset:   660:88fe4866b738
> user:        Daniel Stodden <daniel.stodden@xxxxxxxxxx>
> date:        Wed Oct 07 13:54:16 2009 -0700
> files:       CA-32943-wild-ptr-deref.diff series
> description:
> CA-33070: Fix and reenable my broken CA-30953.diff & co.
> 
> A del_gendisk() definitely wants to find a queue on the disk. Which
> in turn will have dropped to zero right after the cleanup
> call. Because that crackbrained gendisk, as the only queue holder
> which really matters in that entire game, is also the single entity
> left short of maintaining that ref on its own here.
> 
> In summary, it apparently has to be *this* particular order.
> 
> +diff -r ebd0574c414a drivers/xen/blktap/backdev.c
> +--- a/drivers/xen/blktap/backdev.c   Mon Sep 21 16:09:37 2009 -0700
> ++++ b/drivers/xen/blktap/backdev.c   Tue Sep 22 17:16:52 2009 -0700
> +@@ -99,10 +99,9 @@
>       spin_unlock_irq(&backdev_io_lock);
>   
> +     del_gendisk(info->gd);
>  +    blk_cleanup_queue(info->gd->queue);
> -+
> -     del_gendisk(info->gd);
>       put_disk(info->gd);
>   
>  -    blk_cleanup_queue(info->gd->queue);
> 
> 



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