Received: by 10.180.84.234 with SMTP id c10mr3115246wiz.4.1350351348833;
        Mon, 15 Oct 2012 18:35:48 -0700 (PDT)
Path: q10ni65138060wif.0!nntp.google.com!feeder3.cambriumusenet.nl!feed.tweaknews.nl!85.12.40.130.MISMATCH!xlned.com!feeder1.xlned.com!border2.nntp.ams.giganews.com!border3.nntp.ams.giganews.com!border1.nntp.ams.giganews.com!nntp.giganews.com!news.panservice.it!bofh.it!news.nic.it!robomod
From: Jiri Kosina <jkos...@suse.cz>
Newsgroups: linux.kernel
Subject: Re: blk: NULL ptr deref in blk_dequeue_request()
Date: Fri, 12 Oct 2012 17:00:02 +0200
Message-ID: <jTOSu-3KL-23@gated-at.bofh.it>
References: <jMEEx-3re-11@gated-at.bofh.it> <jS3LX-3rZ-3@gated-at.bofh.it> <jSpjs-1mx-25@gated-at.bofh.it> <jStn3-7hD-13@gated-at.bofh.it> <jSI2K-3B6-3@gated-at.bofh.it> <jSI2K-3B6-13@gated-at.bofh.it> <jT6Rt-6oi-17@gated-at.bofh.it>
X-Original-To: Ben Hutchings <b...@decadent.org.uk>,
	Sasha Levin <levinsasha...@gmail.com>
User-Agent: Alpine 2.00 (LNX 1167 2008-08-23)
MIME-Version: 1.0
Sender: robo...@news.nic.it
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Approved: robo...@news.nic.it
Lines: 76
Organization: linux.* mail to news gateway
X-Original-Cc: Jan Kara <j...@suse.cz>, Tejun Heo <t...@kernel.org>,
	Jens Axboe <ax...@kernel.dk>, Dave Jones <da...@redhat.com>,
	"linux-ker...@vger.kernel.org" <linux-ker...@vger.kernel.org>
X-Original-Date: Fri, 12 Oct 2012 16:51:04 +0200 (CEST)
X-Original-Message-ID: <alpine.LNX.2.00.1210121650270.24717@pobox.suse.cz>
X-Original-References: <505E2108.2060...@gmail.com>  <CA+1xoqc1Bc1rPgH+9Xs9-BV7qp3N7mDpNLj1mPwZHdF=H4m...@mail.gmail.com>  <20121008172207.GB14...@quack.suse.cz>  <alpine.LNX.2.00.1210082340501.19...@pobox.suse.cz>  <507424F7.60...@gmail.com> <50742600.10...@gmail.com>
 <1349884374.6336.67.ca...@deadeye.wl.decadent.org.uk>
X-Original-Sender: linux-kernel-ow...@vger.kernel.org
Bytes: 4501
Content-Type: TEXT/PLAIN; charset=US-ASCII

On Wed, 10 Oct 2012, Ben Hutchings wrote:

> > >>>>>> I'm still seeing this on linux-next.
> > >>>>   I think this is floppy related (see redo_fd_request() in the stack
> > >>>> trace). And there were quite some changes to the area recently. Adding
> > >>>> maintainer to CC.
> > >> Hmm ... I don't immediately see how this is happening.
> > >>
> > >> Sasha, could you please do git bisect on drivers/block/floppy.c between 
> > >> f6365201d and your git HEAD for starters (assuming that f6365201d works 
> > >> well for you?).
> > >>
> > > 
> > > A bisect on floppy.c yielded the following:
> > > 
> > > b33d002f4b6bae912463e5a66387c498aa69b6fe is the first bad commit
> > > commit b33d002f4b6bae912463e5a66387c498aa69b6fe
> > > Author: Ben Hutchings <b...@decadent.org.uk>
> > > Date:   Mon Aug 27 20:56:53 2012 -0300
> > > 
> > >     genhd: Make put_disk() safe for disks that have not been registered
> > 
> > 2 more things:
> > 
> >  1. The guest vm which I'm testing on doesn't emulate anything which even looks like a floppy.
> >  2. I'm seeing the following lines before the BUG:
> > 
> > [    9.836604] floppy0: no floppy controllers found
> > [    9.837246] work still pending
> > [    9.837743] floppy0: floppy_shutdown: timeout handler died.
> 
> I see two problems:
> 
> 1. redo_fd_request() races with tear-down of the disks, but because
> set_next_request() checks disk->queue before doing anything this was
> usually harmless.  Now that do_floppy_init() doesn't clear disk->queue,
> the race condition is much easier to hit.  This may fix that problem in
> do_floppy_init(), though there appear to be worse bugs in tear-down
> order in floppy_module_exit():
> 
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -4320,13 +4320,13 @@ out_unreg_region:
>  out_unreg_blkdev:
>  	unregister_blkdev(FLOPPY_MAJOR, "fd");
>  out_put_disk:
> +	destroy_workqueue(floppy_wq);
>  	while (dr--) {
>  		del_timer_sync(&motor_off_timer[dr]);
>  		if (disks[dr]->queue)
>  			blk_cleanup_queue(disks[dr]->queue);
>  		put_disk(disks[dr]);
>  	}
> -	destroy_workqueue(floppy_wq);
>  	return err;
>  }
>  
> --- END ---
> 
> 2. I made a big mistake in using the existing GENHD_FL_UP flag, as it is
> cleared by del_gendisk().  Incremental patch below, but it should be
> squashed into the previous patch if that branch is still rebase-able.

Sasha,

did you manage to test this to see if it fixes the symptom you are seeing, 
please?

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/