On Wed, Aug 08, 2012 at 12:53:04PM -0300, Herton Ronaldo Krzesinski wrote:
> While looking at commit 3f9a5aa (floppy: Cleanup disk->queue before
> caling put_disk() if add_disk() was never called) I noticed some
> problems with the error handling and cleanup:
>
> * missing cleanup (put_disk) if blk_init_queue fails, dr is decremented
> first in the error handling loop
> * if something fails in the add_disk loop, there is no cleanup of
> previous iterations in the error handling.
> * "if (disks[dr]->queue)" check is bogus, when reaching there for each
> dr should exist an queue allocated, and it doesn't take into account
> iterations where add_disk wasn't done, if failure happens in add_disk
> loop.
> * floppy_module_exit doesn't reset queue pointer if add_disk wasn't
> done.
>
> Later commit 4609dff (floppy: Fix a crash during rmmod) fixed the last
> item, this change should handle the remaining problems on floppy_init.
>
> Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
> Cc: [email protected]
> ---
> drivers/block/floppy.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index a7d6347..25ac98b 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4127,6 +4127,7 @@ static int __init do_floppy_init(void)
> {
> int i, unit, drive;
> int err, dr;
> + int registered = -1;
>
> set_debugt();
> interruptjiffies = resultjiffies = jiffies;
> @@ -4153,6 +4154,7 @@ static int __init do_floppy_init(void)
>
> disks[dr]->queue = blk_init_queue(do_fd_request, &floppy_lock);
> if (!disks[dr]->queue) {
> + put_disk(disks[dr]);
> err = -ENOMEM;
> goto out_destroy_workq;
> }
I missed to fix the case for the alloc_ordered_workqueue failure on latest
kernel, I'll resend a fixed version, please ignore this patch for now.
> @@ -4294,7 +4296,7 @@ static int __init do_floppy_init(void)
>
> err = platform_device_register(&floppy_device[drive]);
> if (err)
> - goto out_release_dma;
> + goto out_remove_drives;
>
> err = device_create_file(&floppy_device[drive].dev,
> &dev_attr_cmos);
> @@ -4312,6 +4314,16 @@ static int __init do_floppy_init(void)
>
> out_unreg_platform_dev:
> platform_device_unregister(&floppy_device[drive]);
> +out_remove_drives:
> + registered = drive - 1;
> + while (drive--) {
> + if ((allowed_drive_mask & (1 << drive)) &&
> + fdc_state[FDC(drive)].version != FDC_NONE) {
> + del_gendisk(disks[drive]);
> + device_remove_file(&floppy_device[drive].dev,
> &dev_attr_cmos);
> + platform_device_unregister(&floppy_device[drive]);
> + }
> + }
> out_release_dma:
> if (atomic_read(&usage_count))
> floppy_release_irq_and_dma();
> @@ -4325,14 +4337,14 @@ out_unreg_blkdev:
> out_put_disk:
> while (dr--) {
> del_timer_sync(&motor_off_timer[dr]);
> - if (disks[dr]->queue) {
> - blk_cleanup_queue(disks[dr]->queue);
> - /*
> - * put_disk() is not paired with add_disk() and
> - * will put queue reference one extra time. fix it.
> - */
> + blk_cleanup_queue(disks[dr]->queue);
> + /*
> + * put_disk() may not be paired with add_disk() and
> + * will put queue reference one extra time. fix it.
> + */
> + if (dr > registered || !(allowed_drive_mask & (1 << dr)) ||
> + fdc_state[FDC(dr)].version == FDC_NONE)
> disks[dr]->queue = NULL;
> - }
> put_disk(disks[dr]);
> }
> return err;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
[]'s
Herton
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
