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

[Daniel Stodden <daniel.stodden@xxxxxxxxxx>]

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