openzfs / spl

A shim layer which adds the core interfaces required for OpenZFS.
https://zfsonlinux.org/
GNU General Public License v2.0
282 stars 181 forks source link

HAVE_MUTEX_OWNER checks are misjudged #632

Closed verygreen closed 6 years ago

verygreen commented 6 years ago

I noticed that ZFS has this HAVE_MUTEX_OWNER check and then does stuff to internal in-kernel mutex structures. The problem with that is it's unsafe, for example once you enable lockdep (which I see zfs is prepared to do with the DEBUG_LOCK_ALLOC check hoping that mutex_lock_nested will become a non-GPL exported symbol), kernel would use the owner field in mutex in incompatible ways, in particular once a tainted module is loaded (like zfs), the owner field will always be zero triggering mutex_destroy assertion on zfs right away like this:

[21597.305519] VERIFY3((((volatile typeof((&((&zo->zo_lock)->m_mutex))->owner) )&((&((&zo->zo_lock)->m_mutex))->owner))) == ((void *)0)) failed (ffff88030be28500 == (null)) [21597.307259] PANIC at zfs_onexit.c:104:zfs_onexit_destroy() [21597.307792] Showing stack for process 3626 [21597.308343] CPU: 0 PID: 3626 Comm: mkfs.lustre Tainted: P OE ------------ 3.10.0-debug #1 [21597.309366] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 [21597.309886] ffffffffa046228f 0000000012e37b1d ffff880308c3bcc0 ffffffff816fd404 [21597.310969] ffff880308c3bcd0 ffffffffa01d8754 ffff880308c3be58 ffffffffa01d881f [21597.312000] ffff880308c3bcf0 ffffffff00000030 ffff880308c3be68 ffff880308c3be08 [21597.312984] Call Trace: [21597.313477] [] dump_stack+0x19/0x1b [21597.313993] [] spl_dumpstack+0x44/0x50 [spl] [21597.314529] [] spl_panic+0xbf/0xf0 [spl] [21597.315096] [] ? zfs_onexit_destroy+0x126/0x280 [zfs] [21597.315667] [] zfs_onexit_destroy+0x17c/0x280 [zfs] [21597.316238] [] zfsdev_release+0x48/0xd0 [zfs]

Even if we patch out the taint-disables-lockdep check out in the kernel, this assertion persists, just not as immediate.

As such I believe HAVE_MUTEX_OWNER check needs to be removed and ZFS should retain its own owner field in its own structure for debugging at all times.