Work around vnode reuse bug resulting in a panic: vop_generic_badop

Joel hit this frequently on the go builder, and this was
also found by szykiller

https://syzkaller.appspot.com/bug?extid=58bdde9f7a1a407514a7
https://syzkaller.appspot.com/bug?extid=5779bc64fc4fdd0a5140

This is based on a workaround originally done by visa@ and mbuhl@
but not committed or widely distributed.

Realistically this should be fixed more like the previous attempt
with vdoom, but my attempts to do this at the moment are colliding
with finding all sources of similar races, now that kernel unlocking
is exposing these previously existing bugs

for now, let's put in this ugly workaround

ok deraadt@

diff --git a/sys/ufs/ufs/ufs_ihash.c b/sys/ufs/ufs/ufs_ihash.c
index 758d9a8..b0d6ca3 100644 (file)
--- a/sys/ufs/ufs/ufs_ihash.c
+++ b/sys/ufs/ufs/ufs_ihash.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ufs_ihash.c,v 1.27 2024/07/07 01:39:06 jsg Exp $      */
+/*     $OpenBSD: ufs_ihash.c,v 1.28 2024/09/04 17:00:08 beck Exp $     */
 /*     $NetBSD: ufs_ihash.c,v 1.3 1996/02/09 22:36:04 christos Exp $   */
 
 /*
@@ -36,10 +36,12 @@
 #include <sys/systm.h>
 #include <sys/vnode.h>
 #include <sys/malloc.h>
+#include <sys/mount.h>
 
 #include <ufs/ufs/quota.h>
 #include <ufs/ufs/inode.h>
 #include <ufs/ufs/ufs_extern.h>
+#include <ufs/ufs/ufsmount.h>
 
 #include <crypto/siphash.h>
 
@@ -94,6 +96,33 @@ loop:
                        /* XXXLOCKING unlock hash list? */
                        if (vget(vp, LK_EXCLUSIVE))
                                goto loop;
+                       /*
+                       * Check if the inode is valid.
+                       * The condition has been adapted from ufs_inactive().
+                       *
+                       * This is needed in case our vget above grabbed a vnode
+                       * while ufs_inactive was reclaiming it.
+                       *
+                       * XXX this is a workaround and kind of a gross hack.
+                       * realistically this should get fixed something like
+                       * the previously committed vdoom() or this should be
+                       * dealt with so this can't happen.
+                       */
+                       if (VTOI(vp) != ip ||
+                           (DIP(ip, nlink) <= 0 &&
+                            (vp->v_mount->mnt_flag & MNT_RDONLY) == 0)) {
+                               /*
+                                * This should recycle the inode immediately,
+                                * unless there are other threads that
+                                * try to access it.
+                                * Pause to give the threads a chance to finish
+                                * with the inode.
+                                */
+                               vput(vp);
+                               yield();
+                               goto loop;
+                       }
+
                        return (vp);
                }
        }