From mboxrd@z Thu Jan 1 00:00:00 1970 From: Theodore Tso Subject: Re: [PATCH] fs: Fix mod_timer crash when removing USB sticks Date: Fri, 16 Mar 2012 17:10:21 -0400 Message-ID: References: <1326405431-988-1-git-send-email-taysom@chromium.org> <20120316173610.GA26601@kroah.com> <20120316194300.GA19075@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paul Taysom , Paul Taysom , Mandeep Baines , Jens Axboe , Andrew Morton , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Alexander Viro , linux-fsdevel@vger.kernel.org, stable@kernel.org To: Greg KH Return-path: In-Reply-To: <20120316194300.GA19075@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org I thought another fix at the USB layer also went in that attempted to fix this problem for 3.2, and so with two separate band-aid patches, I think we had thought the problem had been addressed. The real problem is that all of the patches which I've seen to date are band-aids, in that we aren't properly sending a "device as disappeared" notification to the file system layer, but instead we are trying to keep enough of the pointers valid (while also freeing other data structures), such that the file system can blindly write into a partially dismantled block device, and hopefully not oops. Some have argued that my suggested approach of having an explicit super_ops revoke() function, which tells the file system that the block device is gone, etc., isn't necessary because this can be solved in userspace somehow. Personally I think that's nuts, since we'll continue to play whack-a-mole, but I haven't had time to work up patches addressing this --- since this is really only a problem for naive users who pull USB sticks without unmounting them first (and so it never happens to me :-), and I've got a lot of other fish to try..... -- Ted On Fri, Mar 16, 2012 at 3:43 PM, Greg KH wrote: > > On Fri, Mar 16, 2012 at 12:29:15PM -0700, Paul Taysom wrote: > > On Fri, Mar 16, 2012 at 10:36 AM, Greg KH wrote: > > > > > > On Thu, Jan 12, 2012 at 01:57:11PM -0800, Paul Taysom wrote: > > > > A USB stick with a ext file system on it, would occasionally cr= ash > > > > when the stick was pulled. > > > > > > > > The problem was a timer was being set on the Backing Device > > > > Interface, > > > > bdi, after the USB device had been removed and the bdi had been > > > > unregistered. The bdi would then be later reinitialized by zero= ing > > > > the timer without removing from the timer from the timer queue. > > > > This would eventually result in a kernel crash (NULL ptr > > > > dereference). > > > > > > > > When the bdi is unregistered, the dev field is set to NULL. Thi= s > > > > indication is used by bdi_unregister to only unregister the dev= ice > > > > once. > > > > > > > > Fix: When the backing device is invalidated, the mapping > > > > backing_dev_info > > > > should be redirected to the default_backing_dev_info. > > > > > > > > Created 3 USB sticks with ext2, ext4 and one with both apple an= d DOS > > > > file systems on it. Inserted and removed USB sticks many times = in > > > > random > > > > order. With out the bug fix, the kernel would soon crash. With = the > > > > fix, > > > > it did not. Ran on both stumpy and amd64-generic. > > > > > > > > Signed-off-by: Paul Taysom > > > > Cc: Mandeep Baines > > > > Cc: Greg KH > > > > Cc: Jens Axboe > > > > Cc: Theodore Tso > > > > Cc: Andrew Morton > > > > Cc: > > > > Cc: > > > > Cc: Alexander Viro > > > > Cc: > > > > Cc: > > > > --- > > > > =A0fs/block_dev.c | =A0 =A01 + > > > > =A01 files changed, 1 insertions(+), 0 deletions(-) > > > > > > > > diff --git a/fs/block_dev.c b/fs/block_dev.c > > > > index afe74dd..322cd05 100644 > > > > --- a/fs/block_dev.c > > > > +++ b/fs/block_dev.c > > > > @@ -110,6 +110,7 @@ void invalidate_bdev(struct block_device *b= dev) > > > > =A0 =A0 =A0 =A0* But, for the strange corners, lets be cautious > > > > =A0 =A0 =A0 =A0*/ > > > > =A0 =A0 =A0 cleancache_flush_inode(mapping); > > > > + =A0 =A0 mapping->backing_dev_info =3D &default_backing_dev_in= fo; > > > > =A0} > > > > =A0EXPORT_SYMBOL(invalidate_bdev); > > > > > > What ever happened to this patch? =A0Is it still needed? =A0Can y= ou still > > > reproduce the problem on Linus's tree and older kernels? > > > > > > > > > Never heard anything back. =A0Ted supplied a partial fix in 3.2.6 (= I > > believe) for just the ext4 file system. Who should I follow up with= ? > > If the fix went into the 3.2-stable tree, then it's in Linus's tree > already, which is good. > > But, what about all of the other filesystems you hit this on, do we n= eed > to make the same change to all of them? =A0If so, that kind of implie= s > your original patch is the correct one :) > > As for who to poke, Ted, Al, Jens, what should we do here? > > thanks, > > greg k-h