NULL dereference in __blkdev_get() #949
Comments
|
I've seen this reported once or twice as well. I also thought it was probably associated with zvols but you seem to have ruled that out. The really interesting thing here is that we're just calling the user space sys_open() on a normal block device. Nothing in the ZFS kernel stack is involved here. I suppose it's possible we're racing with a concurrent operation on the bdev and things aren't locked properly, but most of that is handled by the kernel. If you have you vmlinux kernel image still around can you resolve the offending line the NULL deref occurred on. That would help narrow down exactly what's getting damaged. gdb /boot/vmlinux list *(__blkdev_get+0x75) |
|
I rebuilt w/ CONFIG_DEBUG_INFO=y but I don't think that would have caused any insns to move about. Assuming gcc didn't shuffle things about much: Line 1137 |
|
No, it's 1137, __blkdev_get + 0x75 == 0xffffffff8112e308 The insb before is from: if (!disk) goto out; blows up: disk not null |
|
Based on the previous code which must have executed to get to this point, and the value of the NULL deref I'm inclined to think bdev is actually a valid address... ahh you just beat me too it. :) |
|
@behlendorf wrt to ERR_PTR theory: &((struct gendisk *) 0ull)->fops = 0x330 the faulting address is 0x2b6, so -116 ERR_PTR would fit that include/asm-generic/errno.h:#define ESTALE 116 /* Stale NFS file handle */ but i don't think i have nfs involved... |
|
@cwedgwood Then the thing to do probably is to add an IS_ERR() check so it doesn't NULL deref and then reproduce the issue under system tap to get a full trace. |
|
get_gendisk-> kobj_lookup()->probe() looks like the most likely candidate. If an ERR_PTR is returned instead of simply NULL on failure we could see this. |
|
so, i put debugging all over the place ... and i can't trigger it anymore i'll leave it in for now on the hope it comes back |
|
What environment are you running this in? We should at least inspect the code for the probe() function, however that depends on what block driver your using. |
|
@behlendorf dual westmere xeon w/ 5 sata disks connected via mpt2sas, ssd for zil, another ssd for l2arc |
|
It appears this would be absolutely possible for a ramdisk block device, see |
|
after recreating the env with copious debugging this race has become hard to hit again if someone has a way to trigger this frequently i would like to know details |
|
@behlendorf doing zfs destroy ... to remove a slew of zvol's i managed to hit this or something similar: context: and i see: then: I had debugging for the later as well but clearly not enough. As mentioned above 00000000000002b6 is smaller than the struct size, so I think this is ERR_PTR + struct offset we're seeing |
|
btw, -2 -> ENOENT |
|
so, in this case it is zvol related, looking at zvol.c we have: now, why zvol_find_by_dev fails i'm not sure (in this it might be that destroyed zvol's didn't do away (i've seen that) so something accessing things tripped up this) pretty sure we can't return ERR_PTR(...) from here though as the rest of the code doesn't like it |
|
Indeed, this does look like an issue in zvol_probe(). We should be returning NULL here instead of an error pointer. |
|
@behlendorf so i made this fix and added some other debugging and of course the race has gone away for now... i'll leave it in and let you know how it goes |
|
update: i can now reproduce the race and with a small fix it does indeed prevent the export/import crash |
|
That's excellent, can you post the patch so we can get it reviewed and merged. This is one of the issues I absolutely wanted fixed in rc12. |
|
@behlendorf let me know how that looks, i can quickly ammend and re-push |
wait many seconds
dmesg:
I thought perhaps there was a race/qurk with zvols, but this happens even in cases where there are none.
The text was updated successfully, but these errors were encountered: