[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/50467: Panic from disconnecting phone while reading its contents
The following reply was made to PR kern/50467; it has been noted by GNATS.
From: David Holland <dholland-bugs%netbsd.org@localhost>
Subject: Re: kern/50467: Panic from disconnecting phone while reading its
Date: Sun, 20 Mar 2016 05:06:51 +0000
On Mon, Feb 15, 2016 at 11:30:01AM +0000, David Holland wrote:
> This dies in spec_strategy:
> KASSERT(vp == vp->v_specnode->sn_dev->sd_bdevvp);
> presumably because the vnode does not match after the device unmounted.
> Instead of KASSERT() you can check for the condition and return EIO...
> Does the kernel go further if you do that?
I'm not sure that's sufficient. Admittedly I don't entirely understand
how specfs is supposed to work; but spec_vnops.c is full of this
assertion and it says it's enforced to prevent the buffer cache from
accumulating multiple inconsistent buffers attached to different
vnodes all linked to the same underlying device.
If sd_bdevvp has become null it's probably ok to fail with EIO, but if
it's some other vnode I think you're in the soup... and once it
becomes null there's AFAIK nothing stopping getting a different vnode
in there if you reinsert and reopen the device.
So I think this condition needs to be blocked earlier. You can't (and
shouldn't) prevent the device from detaching, but you can mark the
vnode dead and/or revoke it and then further attempts to use it won't
get as far as spec_strategy.
But I'm not sure what happens at this level when a device is
detached... it looks like it must be calling spec_close, as there are
only three places sd_bdevvp is set to null in the entire kernel and
two of them are irrelevant. But I don't see how; this requires getting
the open vnode from somewhere (spec_close also enforces the cited
invariant) and there isn't any reasonable way to do that from the
device level. Or so I'd think. Also, theoretically after closing the
open vnode on the block device it shouldn't be possible to use it for
further I/O. Obviously there's something going on here that I'm
David A. Holland
Main Index |
Thread Index |