CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Apr 22 15:32:49 UTC 2023

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: KNF.  No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.217 -r1.218 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.217 src/sys/miscfs/specfs/spec_vnops.c:1.218
--- src/sys/miscfs/specfs/spec_vnops.c:1.217	Sat Apr 22 14:30:16 2023
+++ src/sys/miscfs/specfs/spec_vnops.c	Sat Apr 22 15:32:49 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.217 2023/04/22 14:30:16 hannken Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.218 2023/04/22 15:32:49 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.217 2023/04/22 14:30:16 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.218 2023/04/22 15:32:49 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -184,7 +184,9 @@ const struct vnodeopv_desc spec_vnodeop_
 static kauth_listener_t rawio_listener;
 static struct kcondvar specfs_iocv;
 
-/* Returns true if vnode is /dev/mem or /dev/kmem. */
+/*
+ * Returns true if vnode is /dev/mem or /dev/kmem.
+ */
 bool
 iskmemvp(struct vnode *vp)
 {
@@ -478,7 +480,7 @@ top:	mutex_enter(_lock);
 	}
 	mutex_exit(_lock);
 	error = vcache_vget(vp);
-	if (error != 0)
+	if (error)
 		return error;
 	*vpp = vp;
 
@@ -513,7 +515,7 @@ spec_node_lookup_by_mount(struct mount *
 	mutex_enter(vq->v_interlock);
 	mutex_exit(_lock);
 	error = vcache_vget(vq);
-	if (error != 0)
+	if (error)
 		return error;
 	*vpp = vq;
 
@@ -701,7 +703,7 @@ spec_lookup(void *v)
 	} */ *ap = v;
 
 	*ap->a_vpp = NULL;
-	return (ENOTDIR);
+	return ENOTDIR;
 }
 
 typedef int (*spec_ioctl_t)(dev_t, u_long, void *, int, struct lwp *);
@@ -743,7 +745,7 @@ spec_open(void *v)
 	 * Don't allow open if fs is mounted -nodev.
 	 */
 	if (vp->v_mount && (vp->v_mount->mnt_flag & MNT_NODEV))
-		return (ENXIO);
+		return ENXIO;
 
 	switch (ap->a_mode & (FREAD | FWRITE)) {
 	case FREAD | FWRITE:
@@ -757,8 +759,8 @@ spec_open(void *v)
 		break;
 	}
 	error = kauth_authorize_device_spec(ap->a_cred, req, vp);
-	if (error != 0)
-		return (error);
+	if (error)
+		return error;
 
 	/*
 	 * Acquire an open reference -- as long as we hold onto it, and
@@ -877,7 +879,7 @@ spec_open(void *v)
 			error = cdev_open(dev, ap->a_mode, S_IFCHR, l);
 			if (error != ENXIO)
 break;
-			
+
 			/* Check if we already have a valid driver */
 			mutex_enter(_lock);
 			cdev = cdevsw_lookup(dev);
@@ -888,9 +890,9 @@ spec_open(void *v)
 			/* Get device name from devsw_conv array */
 			if ((name = cdevsw_getname(major(dev))) == NULL)
 break;
-			
+
 			/* Try to autoload device module */
-			(void) module_autoload(name, MODULE_CLASS_DRIVER);
+			(void)module_autoload(name, MODULE_CLASS_DRIVER);
 		} while (gen != module_gen);
 		break;
 
@@ -914,8 +916,8 @@ spec_open(void *v)
 			if ((name = bdevsw_getname(major(dev))) == NULL)
 break;
 
-/* Try to autoload device module */
-			(void) module_autoload(name, MODULE_CLASS_DRIVER);
+			/* Try to autoload device module */
+			(void)module_autoload(name, MODULE_CLASS_DRIVER);
 		} while (gen != module_gen);
 		break;
 
@@ -1062,7 +1064,7 @@ spec_read(void *v)
 	} */ *ap = v;
 	struct vnode *vp = ap->a_vp;
 	struct uio *uio = ap->a_uio;
- 	struct lwp *l = curlwp;
+	struct lwp *l = curlwp;
 	struct specnode *sn;
 	dev_t dev;
 	struct buf *bp;
@@ -1077,12 +1079,12 @@ spec_read(void *v)
 	int nrablks, ratogo;
 
 	KASSERT(uio->uio_rw == UIO_READ);
-	KASSERTMSG(VMSPACE_IS_KERNEL_P(uio->uio_vmspace) ||
-		   uio->uio_vmspace == curproc->p_vmspace,
-		"vmspace belongs to neither kernel nor curproc");
+	KASSERTMSG((VMSPACE_IS_KERNEL_P(uio->uio_vmspace) ||
+		uio->uio_vmspace == curproc->p_vmspace),
+	"vmspace belongs to neither kernel nor curproc");
 
 	if (uio->uio_resid == 0)
-		return (0);
+		return 0;
 
 	switch (vp->v_type) {
 
@@ -1109,12 +,12 @@ spec_read(void *v)
 		spec_io_exit(vp, sn);
 out:		/* XXX What if the caller held an exclusive lock?  */
 		vn_lock(vp, LK_SHARED | LK_RETRY);
-		return (error);
+		return error;
 
 	case VBLK:
 		KASSERT(vp == vp->v_specnode->sn_dev->sd_bdevvp);
 		if (uio->uio_offset < 0)
-			return (EINVAL);
+			return EINVAL;
 
 		if (bdev_ioctl(vp->v_rdev, DIOCGPARTINFO, , FREAD, l) == 0)
 			bsize = imin(imax(pi.pi_bsize, DEV_BSIZE), MAXBSIZE);
@@ -1144,8 +1146,8 @@ out:		/* XXX What if the caller held an 
 }
 
 error = breadn(vp, bn, bsize,
-	   rablks, rasizes, nrablks,
-	   0, );
+rablks, rasizes, nrablks,
+0, );
 			} else {
 if (ratogo > 0)
 	--ratogo;
@@ -1161,7 +1163,7 @@ out:		/* XXX What if the caller 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Apr 22 15:32:49 UTC 2023

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: KNF.  No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.217 -r1.218 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Juergen Hannken-Illjes]

Module Name:src
Committed By:   hannken
Date:   Sat Apr 22 14:30:17 UTC 2023

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
Remove unused specdev member sd_rdev.

Ride 10.99.4


To generate a diff of this commit:
cvs rdiff -u -r1.216 -r1.217 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.53 -r1.54 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.216 src/sys/miscfs/specfs/spec_vnops.c:1.217
--- src/sys/miscfs/specfs/spec_vnops.c:1.216	Sat Oct 15 15:20:46 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Sat Apr 22 14:30:16 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.216 2022/10/15 15:20:46 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.217 2023/04/22 14:30:16 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.216 2022/10/15 15:20:46 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.217 2023/04/22 14:30:16 hannken Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -397,7 +397,6 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 	}
 	if (vp2 == NULL) {
 		/* No existing record, create a new one. */
-		sd->sd_rdev = rdev;
 		sd->sd_mountpoint = NULL;
 		sd->sd_lockf = NULL;
 		sd->sd_refcnt = 1;

Index: src/sys/miscfs/specfs/specdev.h
diff -u src/sys/miscfs/specfs/specdev.h:1.53 src/sys/miscfs/specfs/specdev.h:1.54
--- src/sys/miscfs/specfs/specdev.h:1.53	Wed Oct 26 23:40:08 2022
+++ src/sys/miscfs/specfs/specdev.h	Sat Apr 22 14:30:16 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: specdev.h,v 1.53 2022/10/26 23:40:08 riastradh Exp $	*/
+/*	$NetBSD: specdev.h,v 1.54 2023/04/22 14:30:16 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -77,7 +77,6 @@ typedef struct specdev {
 	vnode_t		*sd_bdevvp;
 	u_int		sd_opencnt;	/* # of opens; close when ->0 */
 	u_int		sd_refcnt;	/* # of specnodes referencing this */
-	dev_t		sd_rdev;
 	volatile u_int	sd_iocnt;	/* # bdev/cdev_* operations active */
 	bool		sd_opened;	/* true if successfully opened */
 	bool		sd_closing;	/* true when bdev/cdev_close ongoing */

CVS commit: src/sys/miscfs/specfs

[Juergen Hannken-Illjes]

Module Name:src
Committed By:   hannken
Date:   Sat Apr 22 14:30:17 UTC 2023

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
Remove unused specdev member sd_rdev.

Ride 10.99.4


To generate a diff of this commit:
cvs rdiff -u -r1.216 -r1.217 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.53 -r1.54 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Oct 15 15:20:46 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs(9): Attribute blame by stack trace for write to r/o medium.


To generate a diff of this commit:
cvs rdiff -u -r1.215 -r1.216 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Oct 15 15:20:46 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs(9): Attribute blame by stack trace for write to r/o medium.


To generate a diff of this commit:
cvs rdiff -u -r1.215 -r1.216 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.215 src/sys/miscfs/specfs/spec_vnops.c:1.216
--- src/sys/miscfs/specfs/spec_vnops.c:1.215	Wed Sep 21 10:59:10 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Sat Oct 15 15:20:46 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.215 2022/09/21 10:59:10 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.216 2022/10/15 15:20:46 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,11 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.215 2022/09/21 10:59:10 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.216 2022/10/15 15:20:46 riastradh Exp $");
+
+#ifdef _KERNEL_OPT
+#include "opt_ddb.h"
+#endif
 
 #include 
 #include 
@@ -86,6 +90,10 @@ __KERNEL_RCSID(0, "$NetBSD: spec_vnops.c
 #include 
 #include 
 
+#ifdef DDB
+#include 
+#endif
+
 /*
  * Lock order:
  *
@@ -1485,6 +1493,9 @@ spec_strategy(void *v)
 			if (mp && (mp->mnt_flag & MNT_RDONLY)) {
 printf("%s blk %"PRId64" written while ro!\n",
 mp->mnt_stat.f_mntonname, bp->b_blkno);
+#ifdef DDB
+db_stacktrace();
+#endif
 			}
 		}
 #endif /* DIAGNOSTIC */

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Wed Sep 21 10:59:10 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs(9): XXX comment: what if read downgrades lock?


To generate a diff of this commit:
cvs rdiff -u -r1.214 -r1.215 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Wed Sep 21 10:59:10 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs(9): XXX comment: what if read downgrades lock?


To generate a diff of this commit:
cvs rdiff -u -r1.214 -r1.215 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.214 src/sys/miscfs/specfs/spec_vnops.c:1.215
--- src/sys/miscfs/specfs/spec_vnops.c:1.214	Fri Aug 12 21:25:39 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Wed Sep 21 10:59:10 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.214 2022/08/12 21:25:39 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.215 2022/09/21 10:59:10 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.214 2022/08/12 21:25:39 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.215 2022/09/21 10:59:10 riastradh Exp $");
 
 #include 
 #include 
@@ -1100,7 +1100,8 @@ spec_read(void *v)
 			goto out;
 		error = cdev_read(dev, uio, ap->a_ioflag);
 		spec_io_exit(vp, sn);
-out:		vn_lock(vp, LK_SHARED | LK_RETRY);
+out:		/* XXX What if the caller held an exclusive lock?  */
+		vn_lock(vp, LK_SHARED | LK_RETRY);
 		return (error);
 
 	case VBLK:

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 21:25:39 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Refuse to open a closing-in-progress block device.

We could wait for close to complete, but if this happened ever so
slightly earlier it would lead to EBUSY anyway, so there's no point
in adding logic for that -- either way the caller neglected to wait
for the last close to finish before trying to open it the device
again.

https://mail-index.netbsd.org/current-users/2022/08/09/msg042800.html

Reported-by: syzbot+4388f20706ec8a4c8...@syzkaller.appspotmail.com
https://syzkaller.appspot.com/bug?id=47c67ab6d3a87514d0707882a9ad6671beaa8642

Reported-by: syzbot+0f1756652dce4cb34...@syzkaller.appspotmail.com
https://syzkaller.appspot.com/bug?id=a632ce762d64241fc82a9bc57230b7b7c7095d1a


To generate a diff of this commit:
cvs rdiff -u -r1.213 -r1.214 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.213 src/sys/miscfs/specfs/spec_vnops.c:1.214
--- src/sys/miscfs/specfs/spec_vnops.c:1.213	Fri Aug 12 17:06:01 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Fri Aug 12 21:25:39 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.213 2022/08/12 17:06:01 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.214 2022/08/12 21:25:39 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.213 2022/08/12 17:06:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.214 2022/08/12 21:25:39 riastradh Exp $");
 
 #include 
 #include 
@@ -789,8 +789,13 @@ spec_open(void *v)
 		 *
 		 * Treat zero opencnt with non-NULL mountpoint as open.
 		 * This may happen after forced detach of a mounted device.
+		 *
+		 * Also treat sd_closing, meaning there is a concurrent
+		 * close in progress, as still open.
 		 */
-		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
+		if (sd->sd_opencnt != 0 ||
+		sd->sd_mountpoint != NULL ||
+		sd->sd_closing) {
 			error = EBUSY;
 			break;
 		}

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 21:25:39 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Refuse to open a closing-in-progress block device.

We could wait for close to complete, but if this happened ever so
slightly earlier it would lead to EBUSY anyway, so there's no point
in adding logic for that -- either way the caller neglected to wait
for the last close to finish before trying to open it the device
again.

https://mail-index.netbsd.org/current-users/2022/08/09/msg042800.html

Reported-by: syzbot+4388f20706ec8a4c8...@syzkaller.appspotmail.com
https://syzkaller.appspot.com/bug?id=47c67ab6d3a87514d0707882a9ad6671beaa8642

Reported-by: syzbot+0f1756652dce4cb34...@syzkaller.appspotmail.com
https://syzkaller.appspot.com/bug?id=a632ce762d64241fc82a9bc57230b7b7c7095d1a


To generate a diff of this commit:
cvs rdiff -u -r1.213 -r1.214 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 17:06:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert !closing on successful open.

- If there's a prior concurrent close, it must have interrupted this
  open.

- If there's a new concurrent close, it must wait until this open has
  released device_lock before it can revoke.


To generate a diff of this commit:
cvs rdiff -u -r1.212 -r1.213 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.212 src/sys/miscfs/specfs/spec_vnops.c:1.213
--- src/sys/miscfs/specfs/spec_vnops.c:1.212	Fri Aug 12 17:05:49 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Fri Aug 12 17:06:01 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.212 2022/08/12 17:05:49 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.213 2022/08/12 17:06:01 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.212 2022/08/12 17:05:49 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.213 2022/08/12 17:06:01 riastradh Exp $");
 
 #include 
 #include 
@@ -967,6 +967,7 @@ spec_open(void *v)
 		KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
 		"sn_opencnt=%u > sd_opencnt=%u",
 		sn->sn_opencnt, sd->sd_opencnt);
+		KASSERT(!sd->sd_closing);
 		sd->sd_opened = true;
 	} else if (sd->sd_opencnt == 1 && sd->sd_opened) {
 		/*

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 17:06:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert !closing on successful open.

- If there's a prior concurrent close, it must have interrupted this
  open.

- If there's a new concurrent close, it must wait until this open has
  released device_lock before it can revoke.


To generate a diff of this commit:
cvs rdiff -u -r1.212 -r1.213 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 17:05:49 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert opencnt>0 on successful open.


To generate a diff of this commit:
cvs rdiff -u -r1.211 -r1.212 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.211 src/sys/miscfs/specfs/spec_vnops.c:1.212
--- src/sys/miscfs/specfs/spec_vnops.c:1.211	Thu Aug 11 12:52:24 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Fri Aug 12 17:05:49 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.211 2022/08/11 12:52:24 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.212 2022/08/12 17:05:49 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.211 2022/08/11 12:52:24 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.212 2022/08/12 17:05:49 riastradh Exp $");
 
 #include 
 #include 
@@ -956,6 +956,17 @@ spec_open(void *v)
 		if (error == 0)
 			error = EBADF;
 	} else if (error == 0) {
+		/*
+		 * Device has not been revoked, so our opencnt can't
+		 * have gone away at this point -- transition to
+		 * sn_gone=true happens before transition to
+		 * sn_opencnt=0 in spec_node_revoke.
+		 */
+		KASSERT(sd->sd_opencnt);
+		KASSERT(sn->sn_opencnt);
+		KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
+		"sn_opencnt=%u > sd_opencnt=%u",
+		sn->sn_opencnt, sd->sd_opencnt);
 		sd->sd_opened = true;
 	} else if (sd->sd_opencnt == 1 && sd->sd_opened) {
 		/*

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Fri Aug 12 17:05:49 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert opencnt>0 on successful open.


To generate a diff of this commit:
cvs rdiff -u -r1.211 -r1.212 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Thu Aug 11 12:52:24 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Sprinkle opencnt/opened/closing assertions.

There seems to be a bug here but I'm not sure what it is yet:

https://mail-index.netbsd.org/current-users/2022/08/09/msg042800.html
https://syzkaller.appspot.com/bug?id=47c67ab6d3a87514d0707882a9ad6671beaa8642

The decision to actually invoke d_close is serialized under
device_lock, so it should not be possible for more than one process
to close at the same time, but syzbot and kre found a way for
sd_closing to be false later in spec_close.  Let's make sure it's
false when we're making what should be the exclusive decision to
close.

We can't assert !sd_opened before cancel and spec_io_drain, because
those are necessary to interrupt and wait for pending opens that
might later set sd_opened, but we can assert !sd_opened afterward
because once sd_closing is true nothing should set sd_opened.


To generate a diff of this commit:
cvs rdiff -u -r1.210 -r1.211 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.210 src/sys/miscfs/specfs/spec_vnops.c:1.211
--- src/sys/miscfs/specfs/spec_vnops.c:1.210	Mon Mar 28 12:39:10 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Thu Aug 11 12:52:24 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.210 2022/03/28 12:39:10 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.211 2022/08/11 12:52:24 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.210 2022/03/28 12:39:10 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.211 2022/08/11 12:52:24 riastradh Exp $");
 
 #include 
 #include 
@@ -603,7 +603,9 @@ spec_node_revoke(vnode_t *vp)
 	KASSERT(sn->sn_gone == false);
 
 	mutex_enter(_lock);
-	KASSERT(sn->sn_opencnt <= sd->sd_opencnt);
+	KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
+	"sn_opencnt=%u > sd_opencnt=%u",
+	sn->sn_opencnt, sd->sd_opencnt);
 	sn->sn_gone = true;
 	if (sn->sn_opencnt != 0) {
 		sd->sd_opencnt -= (sn->sn_opencnt - 1);
@@ -775,6 +777,9 @@ spec_open(void *v)
 			break;
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
+		KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
+		"sn_opencnt=%u > sd_opencnt=%u",
+		sn->sn_opencnt, sd->sd_opencnt);
 		break;
 	case VBLK:
 		/*
@@ -789,7 +794,8 @@ spec_open(void *v)
 			error = EBUSY;
 			break;
 		}
-		KASSERTMSG(sn->sn_opencnt == 0, "%u", sn->sn_opencnt);
+		KASSERTMSG(sn->sn_opencnt == 0, "sn_opencnt=%u",
+		sn->sn_opencnt);
 		sn->sn_opencnt = 1;
 		sd->sd_opencnt = 1;
 		sd->sd_bdevvp = vp;
@@ -963,6 +969,9 @@ spec_open(void *v)
 	} else {
 		KASSERT(sd->sd_opencnt);
 		KASSERT(sn->sn_opencnt);
+		KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
+		"sn_opencnt=%u > sd_opencnt=%u",
+		sn->sn_opencnt, sd->sd_opencnt);
 		sd->sd_opencnt--;
 		sn->sn_opencnt--;
 		if (vp->v_type == VBLK)
@@ -1664,6 +1673,9 @@ spec_close(void *v)
 	mutex_enter(_lock);
 	KASSERT(sn->sn_opencnt);
 	KASSERT(sd->sd_opencnt);
+	KASSERTMSG(sn->sn_opencnt <= sd->sd_opencnt,
+	"sn_opencnt=%u > sd_opencnt=%u",
+	sn->sn_opencnt, sd->sd_opencnt);
 	sn->sn_opencnt--;
 	count = --sd->sd_opencnt;
 	if (vp->v_type == VBLK) {
@@ -1672,6 +1684,9 @@ spec_close(void *v)
 		sd->sd_bdevvp = NULL;
 	}
 	if (count == 0) {
+		KASSERTMSG(sn->sn_opencnt == 0, "sn_opencnt=%u",
+		sn->sn_opencnt);
+		KASSERT(!sd->sd_closing);
 		sd->sd_opened = false;
 		sd->sd_closing = true;
 	}
@@ -1722,6 +1737,7 @@ spec_close(void *v)
 	 * reacquiring the lock would deadlock.
 	 */
 	mutex_enter(_lock);
+	KASSERT(!sd->sd_opened);
 	KASSERT(sd->sd_closing);
 	sd->sd_closing = false;
 	cv_broadcast(_iocv);

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Thu Aug 11 12:52:24 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Sprinkle opencnt/opened/closing assertions.

There seems to be a bug here but I'm not sure what it is yet:

https://mail-index.netbsd.org/current-users/2022/08/09/msg042800.html
https://syzkaller.appspot.com/bug?id=47c67ab6d3a87514d0707882a9ad6671beaa8642

The decision to actually invoke d_close is serialized under
device_lock, so it should not be possible for more than one process
to close at the same time, but syzbot and kre found a way for
sd_closing to be false later in spec_close.  Let's make sure it's
false when we're making what should be the exclusive decision to
close.

We can't assert !sd_opened before cancel and spec_io_drain, because
those are necessary to interrupt and wait for pending opens that
might later set sd_opened, but we can assert !sd_opened afterward
because once sd_closing is true nothing should set sd_opened.


To generate a diff of this commit:
cvs rdiff -u -r1.210 -r1.211 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:38:04 UTC 2022

Modified Files:
src/sys/miscfs/specfs: specdev.h

Log Message:
specfs: Reorder struct specnode members to save padding.

Shrinks from 40 bytes to 32 bytes on LP64 systems this way.


To generate a diff of this commit:
cvs rdiff -u -r1.51 -r1.52 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/specdev.h
diff -u src/sys/miscfs/specfs/specdev.h:1.51 src/sys/miscfs/specfs/specdev.h:1.52
--- src/sys/miscfs/specfs/specdev.h:1.51	Mon Mar 28 12:37:46 2022
+++ src/sys/miscfs/specfs/specdev.h	Mon Mar 28 12:38:04 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: specdev.h,v 1.51 2022/03/28 12:37:46 riastradh Exp $	*/
+/*	$NetBSD: specdev.h,v 1.52 2022/03/28 12:38:04 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -66,8 +66,8 @@
 typedef struct specnode {
 	vnode_t		*sn_next;
 	struct specdev	*sn_dev;
-	u_int		sn_opencnt;	/* # of opens, share of sd_opencnt */
 	dev_t		sn_rdev;
+	u_int		sn_opencnt;	/* # of opens, share of sd_opencnt */
 	bool		sn_gone;
 } specnode_t;
 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:38:04 UTC 2022

Modified Files:
src/sys/miscfs/specfs: specdev.h

Log Message:
specfs: Reorder struct specnode members to save padding.

Shrinks from 40 bytes to 32 bytes on LP64 systems this way.


To generate a diff of this commit:
cvs rdiff -u -r1.51 -r1.52 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:35 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert opencnt is nonzero before decrementing.


To generate a diff of this commit:
cvs rdiff -u -r1.206 -r1.207 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.206 src/sys/miscfs/specfs/spec_vnops.c:1.207
--- src/sys/miscfs/specfs/spec_vnops.c:1.206	Mon Mar 28 12:37:26 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:37:35 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.206 2022/03/28 12:37:26 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.207 2022/03/28 12:37:35 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.206 2022/03/28 12:37:26 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.207 2022/03/28 12:37:35 riastradh Exp $");
 
 #include 
 #include 
@@ -944,6 +944,8 @@ spec_open(void *v)
 		KASSERT(sn->sn_opencnt == 1);
 		needclose = true;
 	} else {
+		KASSERT(sd->sd_opencnt);
+		KASSERT(sn->sn_opencnt);
 		sd->sd_opencnt--;
 		sn->sn_opencnt--;
 		if (vp->v_type == VBLK)
@@ -1643,6 +1645,8 @@ spec_close(void *v)
 	 * between open and close can use fd_clone.
 	 */
 	mutex_enter(_lock);
+	KASSERT(sn->sn_opencnt);
+	KASSERT(sd->sd_opencnt);
 	sn->sn_opencnt--;
 	count = --sd->sd_opencnt;
 	if (vp->v_type == VBLK) {

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:35 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert opencnt is nonzero before decrementing.


To generate a diff of this commit:
cvs rdiff -u -r1.206 -r1.207 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:27 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Take an I/O reference across bdev/cdev_open.

- Revoke is used to invalidate all prior access control checks when
  device permissions are changing, so it must wait for .d_open to exit
  so any new access must go through new access control checks.

- Revoke is used by vdevgone in xyz_detach to wait until all use of
  the driver's data structures have completed before xyz_detach frees
  them.

So we need to make sure spec_close waits for .d_open too.


To generate a diff of this commit:
cvs rdiff -u -r1.205 -r1.206 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.205 src/sys/miscfs/specfs/spec_vnops.c:1.206
--- src/sys/miscfs/specfs/spec_vnops.c:1.205	Mon Mar 28 12:37:18 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:37:26 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.205 2022/03/28 12:37:18 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.206 2022/03/28 12:37:26 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.205 2022/03/28 12:37:18 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.206 2022/03/28 12:37:26 riastradh Exp $");
 
 #include 
 #include 
@@ -694,10 +694,10 @@ spec_open(void *v)
 	} */ *ap = v;
 	struct lwp *l = curlwp;
 	struct vnode *vp = ap->a_vp;
-	dev_t dev;
+	dev_t dev, dev1;
 	int error;
 	enum kauth_device_req req;
-	specnode_t *sn;
+	specnode_t *sn, *sn1;
 	specdev_t *sd;
 	spec_ioctl_t ioctl;
 	u_int gen = 0;
@@ -805,18 +805,34 @@ spec_open(void *v)
 	}
 
 	/*
-	 * Open the device.  If .d_open returns ENXIO (device not
-	 * configured), the driver may not be loaded, so try
-	 * autoloading a module and then try .d_open again if anything
-	 * got loaded.
-	 *
 	 * Because opening the device may block indefinitely, e.g. when
 	 * opening a tty, and loading a module may cross into many
 	 * other subsystems, we must not hold the vnode lock while
 	 * calling .d_open, so release it now and reacquire it when
 	 * done.
+	 *
+	 * Take an I/O reference so that any concurrent spec_close via
+	 * spec_node_revoke will wait for us to finish calling .d_open.
+	 * The vnode can't be dead at this point because we have it
+	 * locked.  Note that if revoked, the driver must interrupt
+	 * .d_open before spec_close starts waiting for I/O to drain so
+	 * this doesn't deadlock.
 	 */
 	VOP_UNLOCK(vp);
+	error = spec_io_enter(vp, , );
+	if (error) {
+		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+		return error;
+	}
+	KASSERT(sn1 == sn);
+	KASSERT(dev1 == dev);
+
+	/*
+	 * Open the device.  If .d_open returns ENXIO (device not
+	 * configured), the driver may not be loaded, so try
+	 * autoloading a module and then try .d_open again if anything
+	 * got loaded.
+	 */
 	switch (vp->v_type) {
 	case VCHR:
 		do {
@@ -871,7 +887,16 @@ spec_open(void *v)
 	default:
 		__unreachable();
 	}
+
+	/*
+	 * Release the I/O reference now that we have called .d_open,
+	 * and reacquire the vnode lock.  At this point, the device may
+	 * have been revoked, so we must tread carefully.  However, sn
+	 * and sd remain valid pointers until we drop our reference.
+	 */
+	spec_io_exit(vp, sn);
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
+	KASSERT(vp->v_specnode == sn);
 
 	/*
 	 * If it has been revoked since we released the vnode lock and

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:27 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Take an I/O reference across bdev/cdev_open.

- Revoke is used to invalidate all prior access control checks when
  device permissions are changing, so it must wait for .d_open to exit
  so any new access must go through new access control checks.

- Revoke is used by vdevgone in xyz_detach to wait until all use of
  the driver's data structures have completed before xyz_detach frees
  them.

So we need to make sure spec_close waits for .d_open too.


To generate a diff of this commit:
cvs rdiff -u -r1.205 -r1.206 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:18 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Wait for last close in spec_node_revoke.

Otherwise, revoke -- and vdevgone, in the detach path of removable
devices -- may complete while I/O operations are still running
concurrently.


To generate a diff of this commit:
cvs rdiff -u -r1.204 -r1.205 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.204 src/sys/miscfs/specfs/spec_vnops.c:1.205
--- src/sys/miscfs/specfs/spec_vnops.c:1.204	Mon Mar 28 12:37:09 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:37:18 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.205 2022/03/28 12:37:18 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.205 2022/03/28 12:37:18 riastradh Exp $");
 
 #include 
 #include 
@@ -597,6 +597,16 @@ spec_node_revoke(vnode_t *vp)
 		mutex_enter(_lock);
 		KASSERT(sn->sn_opencnt == 0);
 	}
+
+	/*
+	 * We may have revoked the vnode in this thread while another
+	 * thread was in the middle of spec_close, in the window when
+	 * spec_close releases the vnode lock to call .d_close for the
+	 * last close.  In that case, wait for the concurrent
+	 * spec_close to complete.
+	 */
+	while (sd->sd_closing)
+		cv_wait(_iocv, _lock);
 	mutex_exit(_lock);
 }
 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:18 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Wait for last close in spec_node_revoke.

Otherwise, revoke -- and vdevgone, in the detach path of removable
devices -- may complete while I/O operations are still running
concurrently.


To generate a diff of this commit:
cvs rdiff -u -r1.204 -r1.205 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:09 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Prevent new opens while close is waiting to drain.

Otherwise, bdev/cdev_close could have cancelled all _existing_ opens,
and waited for them to complete (and freed resources used by them) --
but a new one could start, and hang (e.g., a tty), at the same time
spec_close tries to drain all pending I/O operations, one of which
(the new open) is now hanging indefinitely.

Preventing the new open from even starting until bdev/cdev_close is
finished and all I/O operations have drained avoids this deadlock.


To generate a diff of this commit:
cvs rdiff -u -r1.203 -r1.204 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.49 -r1.50 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.203 src/sys/miscfs/specfs/spec_vnops.c:1.204
--- src/sys/miscfs/specfs/spec_vnops.c:1.203	Mon Mar 28 12:37:01 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:37:09 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.204 2022/03/28 12:37:09 riastradh Exp $");
 
 #include 
 #include 
@@ -397,6 +397,7 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 		sd->sd_bdevvp = NULL;
 		sd->sd_iocnt = 0;
 		sd->sd_opened = false;
+		sd->sd_closing = false;
 		sn->sn_dev = sd;
 		sd = NULL;
 	} else {
@@ -734,8 +735,17 @@ spec_open(void *v)
 	case VCHR:
 		/*
 		 * Character devices can accept opens from multiple
-		 * vnodes.
+		 * vnodes.  But first, wait for any close to finish.
+		 * Wait under the vnode lock so we don't have to worry
+		 * about the vnode being revoked while we wait.
 		 */
+		while (sd->sd_closing) {
+			error = cv_wait_sig(_iocv, _lock);
+			if (error)
+break;
+		}
+		if (error)
+			break;
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
 		break;
@@ -1605,8 +1615,10 @@ spec_close(void *v)
 		count + 1);
 		sd->sd_bdevvp = NULL;
 	}
-	if (count == 0)
+	if (count == 0) {
 		sd->sd_opened = false;
+		sd->sd_closing = true;
+	}
 	mutex_exit(_lock);
 
 	if (count != 0)
@@ -1631,6 +1643,18 @@ spec_close(void *v)
 	 */
 	spec_io_drain(sd);
 
+	/*
+	 * Wake any spec_open calls waiting for close to finish -- do
+	 * this before reacquiring the vnode lock, because spec_open
+	 * holds the vnode lock while waiting, so doing this after
+	 * reacquiring the lock would deadlock.
+	 */
+	mutex_enter(_lock);
+	KASSERT(sd->sd_closing);
+	sd->sd_closing = false;
+	cv_broadcast(_iocv);
+	mutex_exit(_lock);
+
 	if (!(flags & FNONBLOCK))
 		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 

Index: src/sys/miscfs/specfs/specdev.h
diff -u src/sys/miscfs/specfs/specdev.h:1.49 src/sys/miscfs/specfs/specdev.h:1.50
--- src/sys/miscfs/specfs/specdev.h:1.49	Mon Mar 28 12:36:51 2022
+++ src/sys/miscfs/specfs/specdev.h	Mon Mar 28 12:37:09 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: specdev.h,v 1.49 2022/03/28 12:36:51 riastradh Exp $	*/
+/*	$NetBSD: specdev.h,v 1.50 2022/03/28 12:37:09 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -80,6 +80,7 @@ typedef struct specdev {
 	dev_t		sd_rdev;
 	volatile u_int	sd_iocnt;	/* # bdev/cdev_* operations active */
 	bool		sd_opened;	/* true if successfully opened */
+	bool		sd_closing;	/* true when bdev/cdev_close ongoing */
 } specdev_t;
 
 /*

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:09 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Prevent new opens while close is waiting to drain.

Otherwise, bdev/cdev_close could have cancelled all _existing_ opens,
and waited for them to complete (and freed resources used by them) --
but a new one could start, and hang (e.g., a tty), at the same time
spec_close tries to drain all pending I/O operations, one of which
(the new open) is now hanging indefinitely.

Preventing the new open from even starting until bdev/cdev_close is
finished and all I/O operations have drained avoids this deadlock.


To generate a diff of this commit:
cvs rdiff -u -r1.203 -r1.204 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.49 -r1.50 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Take an I/O reference in spec_node_setmountedfs.

This is not quite correct.  We _should_ require the caller to hold a
vnode lock around spec_node_getmountedfs, and an exclusive vnode lock
around spec_node_setmountedfs, so that it is only necessary to check
whether revoke has already happened, not hold an I/O reference.

Unfortunately, various callers in various file systems don't follow
this sensible rule.  So let's at least make sure the vnode can't be
revoked in spec_node_setmountedfs, while we're in bdev_ioctl, and
leave a comment explaining what the sorry state of affairs is and how
to fix it later.


To generate a diff of this commit:
cvs rdiff -u -r1.202 -r1.203 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.202 src/sys/miscfs/specfs/spec_vnops.c:1.203
--- src/sys/miscfs/specfs/spec_vnops.c:1.202	Mon Mar 28 12:36:51 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:37:01 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.202 2022/03/28 12:36:51 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.202 2022/03/28 12:36:51 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.203 2022/03/28 12:37:01 riastradh Exp $");
 
 #include 
 #include 
@@ -498,6 +498,11 @@ spec_node_lookup_by_mount(struct mount *
 
 /*
  * Get the file system mounted on this block device.
+ *
+ * XXX Caller should hold the vnode lock -- shared or exclusive -- so
+ * that this can't changed, and the vnode can't be revoked while we
+ * examine it.  But not all callers do, and they're scattered through a
+ * lot of file systems, so we can't assert this yet.
  */
 struct mount *
 spec_node_getmountedfs(vnode_t *devvp)
@@ -512,23 +517,51 @@ spec_node_getmountedfs(vnode_t *devvp)
 
 /*
  * Set the file system mounted on this block device.
+ *
+ * XXX Caller should hold the vnode lock exclusively so this can't be
+ * changed or assumed by spec_node_getmountedfs while we change it, and
+ * the vnode can't be revoked while we handle it.  But not all callers
+ * do, and they're scattered through a lot of file systems, so we can't
+ * assert this yet.  Instead, for now, we'll take an I/O reference so
+ * at least the ioctl doesn't race with revoke/detach.
+ *
+ * If you do change this to assert an exclusive vnode lock, you must
+ * also do vdead_check before trying bdev_ioctl, because the vnode may
+ * have been revoked by the time the caller locked it, and this is
+ * _not_ a vop -- calls to spec_node_setmountedfs don't go through
+ * v_op, so revoking the vnode doesn't prevent further calls.
+ *
+ * XXX Caller should additionally have the vnode open, at least if mp
+ * is nonnull, but I'm not sure all callers do that -- need to audit.
+ * Currently udf closes the vnode before clearing the mount.
  */
 void
 spec_node_setmountedfs(vnode_t *devvp, struct mount *mp)
 {
 	struct dkwedge_info dkw;
+	struct specnode *sn;
+	dev_t dev;
+	int error;
 
 	KASSERT(devvp->v_type == VBLK);
-	KASSERT(devvp->v_specnode->sn_dev->sd_mountpoint == NULL || mp == NULL);
-	devvp->v_specnode->sn_dev->sd_mountpoint = mp;
-	if (mp == NULL)
-		return;
 
-	if (bdev_ioctl(devvp->v_rdev, DIOCGWEDGEINFO, , FREAD, curlwp) != 0)
+	error = spec_io_enter(devvp, , );
+	if (error)
 		return;
 
+	KASSERT(sn->sn_dev->sd_mountpoint == NULL || mp == NULL);
+	sn->sn_dev->sd_mountpoint = mp;
+	if (mp == NULL)
+		goto out;
+
+	error = bdev_ioctl(dev, DIOCGWEDGEINFO, , FREAD, curlwp);
+	if (error)
+		goto out;
+
 	strlcpy(mp->mnt_stat.f_mntfromlabel, dkw.dkw_wname,
 	sizeof(mp->mnt_stat.f_mntfromlabel));
+
+out:	spec_io_exit(devvp, sn);
 }
 
 /*

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:37:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Take an I/O reference in spec_node_setmountedfs.

This is not quite correct.  We _should_ require the caller to hold a
vnode lock around spec_node_getmountedfs, and an exclusive vnode lock
around spec_node_setmountedfs, so that it is only necessary to check
whether revoke has already happened, not hold an I/O reference.

Unfortunately, various callers in various file systems don't follow
this sensible rule.  So let's at least make sure the vnode can't be
revoked in spec_node_setmountedfs, while we're in bdev_ioctl, and
leave a comment explaining what the sorry state of affairs is and how
to fix it later.


To generate a diff of this commit:
cvs rdiff -u -r1.202 -r1.203 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:51 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Drain all I/O operations after last .d_close call.

New kind of I/O reference on specdevs, sd_iocnt.  This could be done
with psref instead; I chose a reference count instead for now because
we already have to take a per-object lock anyway, v_interlock, for
vdead_check, so another atomic is not likely to hurt much more.  We
can always change the mechanism inside spec_io_enter/exit/drain later
on.

Make sure every access to vp->v_rdev or vp->v_specnode and every call
to a devsw operation is protected either:

- by the vnode lock (with vdead_check if we unlocked/relocked),
- by positive sd_opencnt,
- by spec_io_enter/exit, or
- by sd_opencnt management in open/close.


To generate a diff of this commit:
cvs rdiff -u -r1.201 -r1.202 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.48 -r1.49 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.201 src/sys/miscfs/specfs/spec_vnops.c:1.202
--- src/sys/miscfs/specfs/spec_vnops.c:1.201	Mon Mar 28 12:36:42 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:51 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.201 2022/03/28 12:36:42 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.202 2022/03/28 12:36:51 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.201 2022/03/28 12:36:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.202 2022/03/28 12:36:51 riastradh Exp $");
 
 #include 
 #include 
@@ -81,6 +81,7 @@ __KERNEL_RCSID(0, "$NetBSD: spec_vnops.c
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -173,6 +174,7 @@ const struct vnodeopv_desc spec_vnodeop_
 	{ _vnodeop_p, spec_vnodeop_entries };
 
 static kauth_listener_t rawio_listener;
+static struct kcondvar specfs_iocv;
 
 /* Returns true if vnode is /dev/mem or /dev/kmem. */
 bool
@@ -218,6 +220,141 @@ spec_init(void)
 
 	rawio_listener = kauth_listen_scope(KAUTH_SCOPE_DEVICE,
 	rawio_listener_cb, NULL);
+	cv_init(_iocv, "specio");
+}
+
+/*
+ * spec_io_enter(vp, , )
+ *
+ *	Enter an operation that may not hold vp's vnode lock or an
+ *	fstrans on vp's mount.  Until spec_io_exit, the vnode will not
+ *	be revoked.
+ *
+ *	On success, set sn to the specnode pointer and dev to the dev_t
+ *	number and return zero.  Caller must later call spec_io_exit
+ *	when done.
+ *
+ *	On failure, return ENXIO -- the device has been revoked and no
+ *	longer exists.
+ */
+static int
+spec_io_enter(struct vnode *vp, struct specnode **snp, dev_t *devp)
+{
+	dev_t dev;
+	struct specnode *sn;
+	unsigned iocnt;
+	int error = 0;
+
+	mutex_enter(vp->v_interlock);
+
+	/*
+	 * Extract all the info we need from the vnode, unless the
+	 * vnode has already been reclaimed.  This can happen if the
+	 * underlying device has been removed and all the device nodes
+	 * for it have been revoked.  The caller may not hold a vnode
+	 * lock or fstrans to prevent this from happening before it has
+	 * had an opportunity to notice the vnode is dead.
+	 */
+	if (vdead_check(vp, VDEAD_NOWAIT) != 0 ||
+	(sn = vp->v_specnode) == NULL ||
+	(dev = vp->v_rdev) == NODEV) {
+		error = ENXIO;
+		goto out;
+	}
+
+	/*
+	 * Notify spec_close that we are doing an I/O operation which
+	 * may not be not bracketed by fstrans(9) and thus is not
+	 * blocked by vfs suspension.
+	 *
+	 * We could hold this reference with psref(9) instead, but we
+	 * already have to take the interlock for vdead_check, so
+	 * there's not much more cost here to another atomic operation.
+	 */
+	do {
+		iocnt = atomic_load_relaxed(>sn_dev->sd_iocnt);
+		if (__predict_false(iocnt == UINT_MAX)) {
+			/*
+			 * The I/O count is limited by the number of
+			 * LWPs (which will never overflow this) --
+			 * unless one driver uses another driver via
+			 * specfs, which is rather unusual, but which
+			 * could happen via pud(4) userspace drivers.
+			 * We could use a 64-bit count, but can't use
+			 * atomics for that on all platforms.
+			 * (Probably better to switch to psref or
+			 * localcount instead.)
+			 */
+			error = EBUSY;
+			goto out;
+		}
+	} while (atomic_cas_uint(>sn_dev->sd_iocnt, iocnt, iocnt + 1)
+	!= iocnt);
+
+	/* Success!  */
+	*snp = sn;
+	*devp = dev;
+	error = 0;
+
+out:	mutex_exit(vp->v_interlock);
+	return error;
+}
+
+/*
+ * spec_io_exit(vp, sn)
+ *
+ *	Exit an operation entered with a successful spec_io_enter --
+ *	allow concurrent spec_node_revoke to proceed.  The argument sn
+ *	must match the struct specnode pointer returned by spec_io_exit
+ *	for vp.
+ */
+static void
+spec_io_exit(struct vnode *vp, struct 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:51 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Drain all I/O operations after last .d_close call.

New kind of I/O reference on specdevs, sd_iocnt.  This could be done
with psref instead; I chose a reference count instead for now because
we already have to take a per-object lock anyway, v_interlock, for
vdead_check, so another atomic is not likely to hurt much more.  We
can always change the mechanism inside spec_io_enter/exit/drain later
on.

Make sure every access to vp->v_rdev or vp->v_specnode and every call
to a devsw operation is protected either:

- by the vnode lock (with vdead_check if we unlocked/relocked),
- by positive sd_opencnt,
- by spec_io_enter/exit, or
- by sd_opencnt management in open/close.


To generate a diff of this commit:
cvs rdiff -u -r1.201 -r1.202 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.48 -r1.49 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:42 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Resolve a race between close and a failing reopen.


To generate a diff of this commit:
cvs rdiff -u -r1.200 -r1.201 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.47 -r1.48 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.200 src/sys/miscfs/specfs/spec_vnops.c:1.201
--- src/sys/miscfs/specfs/spec_vnops.c:1.200	Mon Mar 28 12:36:26 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:42 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.200 2022/03/28 12:36:26 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.201 2022/03/28 12:36:42 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.200 2022/03/28 12:36:26 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.201 2022/03/28 12:36:42 riastradh Exp $");
 
 #include 
 #include 
@@ -258,6 +258,7 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 		sd->sd_refcnt = 1;
 		sd->sd_opencnt = 0;
 		sd->sd_bdevvp = NULL;
+		sd->sd_opened = false;
 		sn->sn_dev = sd;
 		sd = NULL;
 	} else {
@@ -518,6 +519,7 @@ spec_open(void *v)
 	spec_ioctl_t ioctl;
 	u_int gen;
 	const char *name;
+	bool needclose = false;
 	struct partinfo pi;
 	
 	l = curlwp;
@@ -688,12 +690,9 @@ spec_open(void *v)
 	 * must fail with EBADF.
 	 *
 	 * Otherwise, if opening it failed, back out and release the
-	 * open reference.
-	 *
-	 * XXX This is wrong -- we might release the last open
-	 * reference here, but we don't close the device.  If only this
-	 * thread's call to open failed, that's fine, but we might
-	 * have:
+	 * open reference.  If it was ever successfully opened and we
+	 * got the last reference this way, it's now our job to close
+	 * it.  This might happen in the following scenario:
 	 *
 	 *	Thread 1		Thread 2
 	 *	VOP_OPEN
@@ -710,21 +709,54 @@ spec_open(void *v)
 	 *	  release vnode lock
 	 *  acquire vnode lock
 	 *  --sd_opencnt == 0
-	 *  but no .d_close (***)
+	 *
+	 * We can't resolve this by making spec_close wait for .d_open
+	 * to complete before examining sd_opencnt, because .d_open can
+	 * hang indefinitely, e.g. for a tty.
 	 */
 	mutex_enter(_lock);
 	if (sn->sn_gone) {
 		if (error == 0)
 			error = EBADF;
-	} else if (error != 0) {
+	} else if (error == 0) {
+		sd->sd_opened = true;
+	} else if (sd->sd_opencnt == 1 && sd->sd_opened) {
+		/*
+		 * We're the last reference to a _previous_ open even
+		 * though this one failed, so we have to close it.
+		 * Don't decrement the reference count here --
+		 * spec_close will do that.
+		 */
+		KASSERT(sn->sn_opencnt == 1);
+		needclose = true;
+	} else {
 		sd->sd_opencnt--;
 		sn->sn_opencnt--;
 		if (vp->v_type == VBLK)
 			sd->sd_bdevvp = NULL;
-
 	}
 	mutex_exit(_lock);
 
+	/*
+	 * If this open failed, but the device was previously opened,
+	 * and another thread concurrently closed the vnode while we
+	 * were in the middle of reopening it, the other thread will
+	 * see sd_opencnt > 0 and thus decide not to call .d_close --
+	 * it is now our responsibility to do so.
+	 *
+	 * XXX The flags passed to VOP_CLOSE here are wrong, but
+	 * drivers can't rely on FREAD|FWRITE anyway -- e.g., consider
+	 * a device opened by thread 0 with O_READ, then opened by
+	 * thread 1 with O_WRITE, then closed by thread 0, and finally
+	 * closed by thread 1; the last .d_close call will have FWRITE
+	 * but not FREAD.  We should just eliminate the FREAD/FWRITE
+	 * parameter to .d_close altogether.
+	 */
+	if (needclose) {
+		KASSERT(error);
+		VOP_CLOSE(vp, FNONBLOCK, NOCRED);
+	}
+
 	/* If anything went wrong, we're done.  */
 	if (error)
 		return error;
@@ -1341,6 +1373,25 @@ spec_close(void *v)
 	 * device.  For block devices, the open reference count must be
 	 * 1 at this point.  If the device's open reference count goes
 	 * to zero, we're the last one out so get the lights.
+	 *
+	 * We may find --sd->sd_opencnt gives zero, and yet
+	 * sd->sd_opened is false.  This happens if the vnode is
+	 * revoked at the same time as it is being opened, which can
+	 * happen when opening a tty blocks indefinitely.  In that
+	 * case, we still must call close -- it is the job of close to
+	 * interrupt the open.  Either way, the device will be no
+	 * longer opened, so we have to clear sd->sd_opened; subsequent
+	 * opens will have responsibility for issuing close.
+	 *
+	 * This has the side effect that the sequence of opens might
+	 * happen out of order -- we might end up doing open, open,
+	 * close, close, instead of open, close, open, close.  This is
+	 * unavoidable with the current devsw API, where open is
+	 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:42 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c specdev.h

Log Message:
specfs: Resolve a race between close and a failing reopen.


To generate a diff of this commit:
cvs rdiff -u -r1.200 -r1.201 src/sys/miscfs/specfs/spec_vnops.c
cvs rdiff -u -r1.47 -r1.48 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:34 UTC 2022

Modified Files:
src/sys/miscfs/specfs: specdev.h

Log Message:
specfs: Document sn_opencnt, sd_opencnt, sd_refcnt.


To generate a diff of this commit:
cvs rdiff -u -r1.46 -r1.47 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/specdev.h
diff -u src/sys/miscfs/specfs/specdev.h:1.46 src/sys/miscfs/specfs/specdev.h:1.47
--- src/sys/miscfs/specfs/specdev.h:1.46	Sun Jul 18 23:57:15 2021
+++ src/sys/miscfs/specfs/specdev.h	Mon Mar 28 12:36:34 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: specdev.h,v 1.46 2021/07/18 23:57:15 dholland Exp $	*/
+/*	$NetBSD: specdev.h,v 1.47 2022/03/28 12:36:34 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -66,7 +66,7 @@
 typedef struct specnode {
 	vnode_t		*sn_next;
 	struct specdev	*sn_dev;
-	u_int		sn_opencnt;
+	u_int		sn_opencnt;	/* # of opens, share of sd_opencnt */
 	dev_t		sn_rdev;
 	bool		sn_gone;
 } specnode_t;
@@ -75,8 +75,8 @@ typedef struct specdev {
 	struct mount	*sd_mountpoint;
 	struct lockf	*sd_lockf;
 	vnode_t		*sd_bdevvp;
-	u_int		sd_opencnt;
-	u_int		sd_refcnt;
+	u_int		sd_opencnt;	/* # of opens; close when ->0 */
+	u_int		sd_refcnt;	/* # of specnodes referencing this */
 	dev_t		sd_rdev;
 } specdev_t;
 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:34 UTC 2022

Modified Files:
src/sys/miscfs/specfs: specdev.h

Log Message:
specfs: Document sn_opencnt, sd_opencnt, sd_refcnt.


To generate a diff of this commit:
cvs rdiff -u -r1.46 -r1.47 src/sys/miscfs/specfs/specdev.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:27 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Paranoia: Assert opencnt is zero on reclaim.


To generate a diff of this commit:
cvs rdiff -u -r1.199 -r1.200 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.199 src/sys/miscfs/specfs/spec_vnops.c:1.200
--- src/sys/miscfs/specfs/spec_vnops.c:1.199	Mon Mar 28 12:36:18 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:26 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.199 2022/03/28 12:36:18 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.200 2022/03/28 12:36:26 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.199 2022/03/28 12:36:18 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.200 2022/03/28 12:36:26 riastradh Exp $");
 
 #include 
 #include 
@@ -1204,6 +1204,8 @@ spec_reclaim(void *v)
 	} */ *ap = v;
 	struct vnode *vp = ap->a_vp;
 
+	KASSERT(vp->v_specnode->sn_opencnt == 0);
+
 	VOP_UNLOCK(vp);
 
 	KASSERT(vp->v_mount == dead_rootmount);

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:27 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Paranoia: Assert opencnt is zero on reclaim.


To generate a diff of this commit:
cvs rdiff -u -r1.199 -r1.200 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:18 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Omit needless vdead_check in spec_fdiscard.

The vnode lock is held, so the vnode cannot be revoked without also
changing v_op so subsequent uses under the vnode lock will go to
deadfs's VOP_FDISCARD instead (which is genfs_eopnotsupp).


To generate a diff of this commit:
cvs rdiff -u -r1.198 -r1.199 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.198 src/sys/miscfs/specfs/spec_vnops.c:1.199
--- src/sys/miscfs/specfs/spec_vnops.c:1.198	Mon Mar 28 12:36:09 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:18 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.198 2022/03/28 12:36:09 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.199 2022/03/28 12:36:18 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.198 2022/03/28 12:36:09 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.199 2022/03/28 12:36:18 riastradh Exp $");
 
 #include 
 #include 
@@ -945,17 +945,7 @@ spec_fdiscard(void *v)
 	dev_t dev;
 
 	vp = ap->a_vp;
-	dev = NODEV;
-
-	mutex_enter(vp->v_interlock);
-	if (vdead_check(vp, VDEAD_NOWAIT) == 0 && vp->v_specnode != NULL) {
-		dev = vp->v_rdev;
-	}
-	mutex_exit(vp->v_interlock);
-
-	if (dev == NODEV) {
-		return ENXIO;
-	}
+	dev = vp->v_rdev;
 
 	switch (vp->v_type) {
 	case VCHR:

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:18 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Omit needless vdead_check in spec_fdiscard.

The vnode lock is held, so the vnode cannot be revoked without also
changing v_op so subsequent uses under the vnode lock will go to
deadfs's VOP_FDISCARD instead (which is genfs_eopnotsupp).


To generate a diff of this commit:
cvs rdiff -u -r1.198 -r1.199 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:09 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Add a comment and assertion to spec_close about refcnts.


To generate a diff of this commit:
cvs rdiff -u -r1.197 -r1.198 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:09 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Add a comment and assertion to spec_close about refcnts.


To generate a diff of this commit:
cvs rdiff -u -r1.197 -r1.198 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.197 src/sys/miscfs/specfs/spec_vnops.c:1.198
--- src/sys/miscfs/specfs/spec_vnops.c:1.197	Mon Mar 28 12:36:00 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:09 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.197 2022/03/28 12:36:00 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.198 2022/03/28 12:36:09 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.197 2022/03/28 12:36:00 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.198 2022/03/28 12:36:09 riastradh Exp $");
 
 #include 
 #include 
@@ -1344,11 +1344,20 @@ spec_close(void *v)
 		panic("spec_close: not special");
 	}
 
+	/*
+	 * Decrement the open reference count of this node and the
+	 * device.  For block devices, the open reference count must be
+	 * 1 at this point.  If the device's open reference count goes
+	 * to zero, we're the last one out so get the lights.
+	 */
 	mutex_enter(_lock);
 	sn->sn_opencnt--;
 	count = --sd->sd_opencnt;
-	if (vp->v_type == VBLK)
+	if (vp->v_type == VBLK) {
+		KASSERTMSG(count == 0, "block device with %u opens",
+		count + 1);
 		sd->sd_bdevvp = NULL;
+	}
 	mutex_exit(_lock);
 
 	if (count != 0)

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: If sd_opencnt is zero, sn_opencnt had better be zero.


To generate a diff of this commit:
cvs rdiff -u -r1.196 -r1.197 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.196 src/sys/miscfs/specfs/spec_vnops.c:1.197
--- src/sys/miscfs/specfs/spec_vnops.c:1.196	Mon Mar 28 12:35:52 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:36:00 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.196 2022/03/28 12:35:52 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.197 2022/03/28 12:36:00 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.196 2022/03/28 12:35:52 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.197 2022/03/28 12:36:00 riastradh Exp $");
 
 #include 
 #include 
@@ -581,6 +581,7 @@ spec_open(void *v)
 			error = EBUSY;
 			break;
 		}
+		KASSERTMSG(sn->sn_opencnt == 0, "%u", sn->sn_opencnt);
 		sn->sn_opencnt = 1;
 		sd->sd_opencnt = 1;
 		sd->sd_bdevvp = vp;

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:36:01 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: If sd_opencnt is zero, sn_opencnt had better be zero.


To generate a diff of this commit:
cvs rdiff -u -r1.196 -r1.197 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:52 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor KASSERT out of switch in spec_open.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.195 -r1.196 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.195 src/sys/miscfs/specfs/spec_vnops.c:1.196
--- src/sys/miscfs/specfs/spec_vnops.c:1.195	Mon Mar 28 12:35:44 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:52 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.195 2022/03/28 12:35:44 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.196 2022/03/28 12:35:52 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.195 2022/03/28 12:35:44 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.196 2022/03/28 12:35:52 riastradh Exp $");
 
 #include 
 #include 
@@ -558,13 +558,13 @@ spec_open(void *v)
 	 * can't be revoked until we release the vnode lock.
 	 */
 	mutex_enter(_lock);
+	KASSERT(!sn->sn_gone);
 	switch (vp->v_type) {
 	case VCHR:
 		/*
 		 * Character devices can accept opens from multiple
 		 * vnodes.
 		 */
-		KASSERT(!sn->sn_gone);
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
 		break;
@@ -577,7 +577,6 @@ spec_open(void *v)
 		 * Treat zero opencnt with non-NULL mountpoint as open.
 		 * This may happen after forced detach of a mounted device.
 		 */
-		KASSERT(!sn->sn_gone);
 		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
 			error = EBUSY;
 			break;

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:52 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor KASSERT out of switch in spec_open.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.195 -r1.196 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:44 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: sn_gone cannot be set while we hold the vnode lock.

Revoke runs with the vnode lock too, which is exclusive.  Add an
assertion to this effect in spec_node_revoke to make it clear.


To generate a diff of this commit:
cvs rdiff -u -r1.194 -r1.195 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.194 src/sys/miscfs/specfs/spec_vnops.c:1.195
--- src/sys/miscfs/specfs/spec_vnops.c:1.194	Mon Mar 28 12:35:35 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:44 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.194 2022/03/28 12:35:35 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.195 2022/03/28 12:35:44 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.194 2022/03/28 12:35:35 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.195 2022/03/28 12:35:44 riastradh Exp $");
 
 #include 
 #include 
@@ -402,6 +402,8 @@ spec_node_revoke(vnode_t *vp)
 	specnode_t *sn;
 	specdev_t *sd;
 
+	KASSERT(VOP_ISLOCKED(vp) == LK_EXCLUSIVE);
+
 	sn = vp->v_specnode;
 	sd = sn->sn_dev;
 
@@ -552,11 +554,8 @@ spec_open(void *v)
 
 	/*
 	 * Acquire an open reference -- as long as we hold onto it, and
-	 * the vnode isn't revoked, it can't be closed.
-	 *
-	 * But first check whether it has been revoked -- if so, we
-	 * can't acquire more open references and we must fail
-	 * immediately with EBADF.
+	 * the vnode isn't revoked, it can't be closed, and the vnode
+	 * can't be revoked until we release the vnode lock.
 	 */
 	mutex_enter(_lock);
 	switch (vp->v_type) {
@@ -565,10 +564,7 @@ spec_open(void *v)
 		 * Character devices can accept opens from multiple
 		 * vnodes.
 		 */
-		if (sn->sn_gone) {
-			error = EBADF;
-			break;
-		}
+		KASSERT(!sn->sn_gone);
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
 		break;
@@ -581,10 +577,7 @@ spec_open(void *v)
 		 * Treat zero opencnt with non-NULL mountpoint as open.
 		 * This may happen after forced detach of a mounted device.
 		 */
-		if (sn->sn_gone) {
-			error = EBADF;
-			break;
-		}
+		KASSERT(!sn->sn_gone);
 		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
 			error = EBUSY;
 			break;

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:44 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: sn_gone cannot be set while we hold the vnode lock.

Revoke runs with the vnode lock too, which is exclusive.  Add an
assertion to this effect in spec_node_revoke to make it clear.


To generate a diff of this commit:
cvs rdiff -u -r1.194 -r1.195 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:35 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Reorganize D_DISK tail of spec_open and explain what's up.

No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.193 -r1.194 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.193 src/sys/miscfs/specfs/spec_vnops.c:1.194
--- src/sys/miscfs/specfs/spec_vnops.c:1.193	Mon Mar 28 12:35:26 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:35 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.193 2022/03/28 12:35:26 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.194 2022/03/28 12:35:35 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.193 2022/03/28 12:35:26 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.194 2022/03/28 12:35:35 riastradh Exp $");
 
 #include 
 #include 
@@ -732,15 +732,27 @@ spec_open(void *v)
 	}
 	mutex_exit(_lock);
 
-	if (cdev_type(dev) != D_DISK || error != 0)
+	/* If anything went wrong, we're done.  */
+	if (error)
 		return error;
 
-	
-	ioctl = vp->v_type == VCHR ? cdev_ioctl : bdev_ioctl;
-	error = (*ioctl)(vp->v_rdev, DIOCGPARTINFO, , FREAD, curlwp);
-	if (error == 0)
-		uvm_vnp_setsize(vp, (voff_t)pi.pi_secsize * pi.pi_size);
+	/*
+	 * For disk devices, automagically set the vnode size to the
+	 * partition size, if we can.  This applies to block devices
+	 * and character devices alike -- every block device must have
+	 * a corresponding character device.  And if the module is
+	 * loaded it will remain loaded until we're done here (it is
+	 * forbidden to devsw_detach until closed).  So it is safe to
+	 * query cdev_type unconditionally here.
+	 */
+	if (cdev_type(dev) == D_DISK) {
+		ioctl = vp->v_type == VCHR ? cdev_ioctl : bdev_ioctl;
+		if ((*ioctl)(dev, DIOCGPARTINFO, , FREAD, curlwp) == 0)
+			uvm_vnp_setsize(vp,
+			(voff_t)pi.pi_secsize * pi.pi_size);
+	}
 
+	/* Success!  */
 	return 0;
 }
 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:35 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Reorganize D_DISK tail of spec_open and explain what's up.

No functional change intended.


To generate a diff of this commit:
cvs rdiff -u -r1.193 -r1.194 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:26 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor VOP_UNLOCK/vn_lock out of switch for clarity.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.192 -r1.193 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.192 src/sys/miscfs/specfs/spec_vnops.c:1.193
--- src/sys/miscfs/specfs/spec_vnops.c:1.192	Mon Mar 28 12:35:17 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:26 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.192 2022/03/28 12:35:17 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.193 2022/03/28 12:35:26 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.192 2022/03/28 12:35:17 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.193 2022/03/28 12:35:26 riastradh Exp $");
 
 #include 
 #include 
@@ -632,9 +632,9 @@ spec_open(void *v)
 	 * calling .d_open, so release it now and reacquire it when
 	 * done.
 	 */
+	VOP_UNLOCK(vp);
 	switch (vp->v_type) {
 	case VCHR:
-		VOP_UNLOCK(vp);
 		do {
 			const struct cdevsw *cdev;
 
@@ -657,12 +657,9 @@ spec_open(void *v)
 			/* Try to autoload device module */
 			(void) module_autoload(name, MODULE_CLASS_DRIVER);
 		} while (gen != module_gen);
-
-		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 		break;
 
 	case VBLK:
-		VOP_UNLOCK(vp);
 		do {
 			const struct bdevsw *bdev;
 
@@ -685,13 +682,12 @@ spec_open(void *v)
 /* Try to autoload device module */
 			(void) module_autoload(name, MODULE_CLASS_DRIVER);
 		} while (gen != module_gen);
-		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-
 		break;
 
 	default:
 		__unreachable();
 	}
+	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 
 	/*
 	 * If it has been revoked since we released the vnode lock and

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:26 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor VOP_UNLOCK/vn_lock out of switch for clarity.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.192 -r1.193 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:17 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor common device_lock out of switch for clarity.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.191 -r1.192 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.191 src/sys/miscfs/specfs/spec_vnops.c:1.192
--- src/sys/miscfs/specfs/spec_vnops.c:1.191	Mon Mar 28 12:35:08 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:17 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.191 2022/03/28 12:35:08 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.192 2022/03/28 12:35:17 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.191 2022/03/28 12:35:08 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.192 2022/03/28 12:35:17 riastradh Exp $");
 
 #include 
 #include 
@@ -558,20 +558,19 @@ spec_open(void *v)
 	 * can't acquire more open references and we must fail
 	 * immediately with EBADF.
 	 */
+	mutex_enter(_lock);
 	switch (vp->v_type) {
 	case VCHR:
 		/*
 		 * Character devices can accept opens from multiple
 		 * vnodes.
 		 */
-		mutex_enter(_lock);
 		if (sn->sn_gone) {
-			mutex_exit(_lock);
-			return (EBADF);
+			error = EBADF;
+			break;
 		}
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
-		mutex_exit(_lock);
 		break;
 	case VBLK:
 		/*
@@ -582,23 +581,24 @@ spec_open(void *v)
 		 * Treat zero opencnt with non-NULL mountpoint as open.
 		 * This may happen after forced detach of a mounted device.
 		 */
-		mutex_enter(_lock);
 		if (sn->sn_gone) {
-			mutex_exit(_lock);
-			return (EBADF);
+			error = EBADF;
+			break;
 		}
 		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
-			mutex_exit(_lock);
-			return EBUSY;
+			error = EBUSY;
+			break;
 		}
 		sn->sn_opencnt = 1;
 		sd->sd_opencnt = 1;
 		sd->sd_bdevvp = vp;
-		mutex_exit(_lock);
 		break;
 	default:
 		panic("invalid specfs vnode type: %d", vp->v_type);
 	}
+	mutex_exit(_lock);
+	if (error)
+		return error;
 
 	/*
 	 * Set VV_ISTTY if this is a tty cdev.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:17 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor common device_lock out of switch for clarity.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.191 -r1.192 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:08 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Delete bogus comment about .d_open/.d_close at same time.

Annoying as it is that .d_open and .d_close can run at the same time,
it is also necessary for tty semantics, where open can block
indefinitely, and it is the responsibility of close (called via
revoke) necessary to interrupt it.


To generate a diff of this commit:
cvs rdiff -u -r1.190 -r1.191 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.190 src/sys/miscfs/specfs/spec_vnops.c:1.191
--- src/sys/miscfs/specfs/spec_vnops.c:1.190	Mon Mar 28 12:34:59 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:35:08 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.190 2022/03/28 12:34:59 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.191 2022/03/28 12:35:08 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.190 2022/03/28 12:34:59 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.191 2022/03/28 12:35:08 riastradh Exp $");
 
 #include 
 #include 
@@ -557,12 +557,6 @@ spec_open(void *v)
 	 * But first check whether it has been revoked -- if so, we
 	 * can't acquire more open references and we must fail
 	 * immediately with EBADF.
-	 *
-	 * XXX This races with revoke: once we release the vnode lock,
-	 * the vnode may be revoked, and the .d_close callback run, at
-	 * the same time as we're calling .d_open here.  Drivers
-	 * shouldn't have to contemplate this scenario; .d_open and
-	 * .d_close should be prevented from running concurrently.
 	 */
 	switch (vp->v_type) {
 	case VCHR:

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:35:08 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Delete bogus comment about .d_open/.d_close at same time.

Annoying as it is that .d_open and .d_close can run at the same time,
it is also necessary for tty semantics, where open can block
indefinitely, and it is the responsibility of close (called via
revoke) necessary to interrupt it.


To generate a diff of this commit:
cvs rdiff -u -r1.190 -r1.191 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:59 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Split spec_open switch into three sections.

The sections are now:

1. Acquire open reference.

1a (intermezzo). Set VV_ISTTY.

2. Drop the vnode lock to call .d_open and autoload modules if
   necessary.

3. Handle concurrent revoke if it happenend, or release open reference
   if .d_open failed.

No functional change.  Sprinkle comments about problems.


To generate a diff of this commit:
cvs rdiff -u -r1.189 -r1.190 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.189 src/sys/miscfs/specfs/spec_vnops.c:1.190
--- src/sys/miscfs/specfs/spec_vnops.c:1.189	Mon Mar 28 12:34:51 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:34:59 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.189 2022/03/28 12:34:51 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.190 2022/03/28 12:34:59 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.189 2022/03/28 12:34:51 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.190 2022/03/28 12:34:59 riastradh Exp $");
 
 #include 
 #include 
@@ -550,6 +550,20 @@ spec_open(void *v)
 	if (error != 0)
 		return (error);
 
+	/*
+	 * Acquire an open reference -- as long as we hold onto it, and
+	 * the vnode isn't revoked, it can't be closed.
+	 *
+	 * But first check whether it has been revoked -- if so, we
+	 * can't acquire more open references and we must fail
+	 * immediately with EBADF.
+	 *
+	 * XXX This races with revoke: once we release the vnode lock,
+	 * the vnode may be revoked, and the .d_close callback run, at
+	 * the same time as we're calling .d_open here.  Drivers
+	 * shouldn't have to contemplate this scenario; .d_open and
+	 * .d_close should be prevented from running concurrently.
+	 */
 	switch (vp->v_type) {
 	case VCHR:
 		/*
@@ -564,8 +578,68 @@ spec_open(void *v)
 		sd->sd_opencnt++;
 		sn->sn_opencnt++;
 		mutex_exit(_lock);
+		break;
+	case VBLK:
+		/*
+		 * For block devices, permit only one open.  The buffer
+		 * cache cannot remain self-consistent with multiple
+		 * vnodes holding a block device open.
+		 *
+		 * Treat zero opencnt with non-NULL mountpoint as open.
+		 * This may happen after forced detach of a mounted device.
+		 */
+		mutex_enter(_lock);
+		if (sn->sn_gone) {
+			mutex_exit(_lock);
+			return (EBADF);
+		}
+		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
+			mutex_exit(_lock);
+			return EBUSY;
+		}
+		sn->sn_opencnt = 1;
+		sd->sd_opencnt = 1;
+		sd->sd_bdevvp = vp;
+		mutex_exit(_lock);
+		break;
+	default:
+		panic("invalid specfs vnode type: %d", vp->v_type);
+	}
+
+	/*
+	 * Set VV_ISTTY if this is a tty cdev.
+	 *
+	 * XXX This does the wrong thing if the module has to be
+	 * autoloaded.  We should maybe set this after autoloading
+	 * modules and calling .d_open successfully, except (a) we need
+	 * the vnode lock to touch it, and (b) once we acquire the
+	 * vnode lock again, the vnode may have been revoked, and
+	 * deadfs's dead_read needs VV_ISTTY to be already set in order
+	 * to return the right answer.  So this needs some additional
+	 * synchronization to be made to work correctly with tty driver
+	 * module autoload.  For now, let's just hope it doesn't cause
+	 * too much trouble for a tty from an autoloaded driver module
+	 * to fail with EIO instead of returning EOF.
+	 */
+	if (vp->v_type == VCHR) {
 		if (cdev_type(dev) == D_TTY)
 			vp->v_vflag |= VV_ISTTY;
+	}
+
+	/*
+	 * Open the device.  If .d_open returns ENXIO (device not
+	 * configured), the driver may not be loaded, so try
+	 * autoloading a module and then try .d_open again if anything
+	 * got loaded.
+	 *
+	 * Because opening the device may block indefinitely, e.g. when
+	 * opening a tty, and loading a module may cross into many
+	 * other subsystems, we must not hold the vnode lock while
+	 * calling .d_open, so release it now and reacquire it when
+	 * done.
+	 */
+	switch (vp->v_type) {
+	case VCHR:
 		VOP_UNLOCK(vp);
 		do {
 			const struct cdevsw *cdev;
@@ -594,27 +668,6 @@ spec_open(void *v)
 		break;
 
 	case VBLK:
-		/*
-		 * For block devices, permit only one open.  The buffer
-		 * cache cannot remain self-consistent with multiple
-		 * vnodes holding a block device open.
-		 *
-		 * Treat zero opencnt with non-NULL mountpoint as open.
-		 * This may happen after forced detach of a mounted device.
-		 */
-		mutex_enter(_lock);
-		if (sn->sn_gone) {
-			mutex_exit(_lock);
-			return (EBADF);
-		}
-		if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) {
-			mutex_exit(_lock);
-			return EBUSY;
-		}
-		sn->sn_opencnt = 1;
-		

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:59 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Split spec_open switch into three sections.

The sections are now:

1. Acquire open reference.

1a (intermezzo). Set VV_ISTTY.

2. Drop the vnode lock to call .d_open and autoload modules if
   necessary.

3. Handle concurrent revoke if it happenend, or release open reference
   if .d_open failed.

No functional change.  Sprinkle comments about problems.


To generate a diff of this commit:
cvs rdiff -u -r1.189 -r1.190 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:51 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor common kauth check out of switch in spec_open.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.188 -r1.189 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.188 src/sys/miscfs/specfs/spec_vnops.c:1.189
--- src/sys/miscfs/specfs/spec_vnops.c:1.188	Mon Mar 28 12:34:42 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:34:51 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.188 2022/03/28 12:34:42 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.189 2022/03/28 12:34:51 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.188 2022/03/28 12:34:42 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.189 2022/03/28 12:34:51 riastradh Exp $");
 
 #include 
 #include 
@@ -546,13 +546,12 @@ spec_open(void *v)
 		req = KAUTH_REQ_DEVICE_RAWIO_SPEC_READ;
 		break;
 	}
+	error = kauth_authorize_device_spec(ap->a_cred, req, vp);
+	if (error != 0)
+		return (error);
 
 	switch (vp->v_type) {
 	case VCHR:
-		error = kauth_authorize_device_spec(ap->a_cred, req, vp);
-		if (error != 0)
-			return (error);
-
 		/*
 		 * Character devices can accept opens from multiple
 		 * vnodes.
@@ -595,10 +594,6 @@ spec_open(void *v)
 		break;
 
 	case VBLK:
-		error = kauth_authorize_device_spec(ap->a_cred, req, vp);
-		if (error != 0)
-			return (error);
-
 		/*
 		 * For block devices, permit only one open.  The buffer
 		 * cache cannot remain self-consistent with multiple

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:51 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Factor common kauth check out of switch in spec_open.

No functional change.


To generate a diff of this commit:
cvs rdiff -u -r1.188 -r1.189 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:42 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert v_type is VBLK or VCHR in spec_open.

Nothing else makes sense.  Prune dead branches (and replace default
case by panic).


To generate a diff of this commit:
cvs rdiff -u -r1.187 -r1.188 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.187 src/sys/miscfs/specfs/spec_vnops.c:1.188
--- src/sys/miscfs/specfs/spec_vnops.c:1.187	Mon Mar 28 12:34:34 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:34:42 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.187 2022/03/28 12:34:34 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.188 2022/03/28 12:34:42 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.187 2022/03/28 12:34:34 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.188 2022/03/28 12:34:42 riastradh Exp $");
 
 #include 
 #include 
@@ -525,7 +525,10 @@ spec_open(void *v)
 	sd = sn->sn_dev;
 	name = NULL;
 	gen = 0;
-	
+
+	KASSERTMSG(vp->v_type == VBLK || vp->v_type == VCHR, "type=%d",
+	vp->v_type);
+
 	/*
 	 * Don't allow open if fs is mounted -nodev.
 	 */
@@ -644,15 +647,8 @@ spec_open(void *v)
 
 		break;
 
-	case VNON:
-	case VLNK:
-	case VDIR:
-	case VREG:
-	case VBAD:
-	case VFIFO:
-	case VSOCK:
 	default:
-		return 0;
+		panic("invalid specfs vnode type: %d", vp->v_type);
 	}
 
 	mutex_enter(_lock);

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:42 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Assert v_type is VBLK or VCHR in spec_open.

Nothing else makes sense.  Prune dead branches (and replace default
case by panic).


To generate a diff of this commit:
cvs rdiff -u -r1.187 -r1.188 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:34 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Call bdev_open without the vnode lock.

There is no need for it to serialize opens, because they are already
serialized by sd_opencnt which for block devices is always either 0
or 1.

There's not obviously any other reason why the vnode lock should be
held across bdev_open, other than that it might be nice to avoid
dropping it if not necessary.  For character devices we always have
to drop the vnode lock because open might hang indefinitely, when
opening a tty, which is not allowed while holding the vnode lock.


To generate a diff of this commit:
cvs rdiff -u -r1.186 -r1.187 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.186 src/sys/miscfs/specfs/spec_vnops.c:1.187
--- src/sys/miscfs/specfs/spec_vnops.c:1.186	Mon Mar 28 12:34:25 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:34:34 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.186 2022/03/28 12:34:25 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.187 2022/03/28 12:34:34 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.186 2022/03/28 12:34:25 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.187 2022/03/28 12:34:34 riastradh Exp $");
 
 #include 
 #include 
@@ -617,6 +617,7 @@ spec_open(void *v)
 		sd->sd_opencnt = 1;
 		sd->sd_bdevvp = vp;
 		mutex_exit(_lock);
+		VOP_UNLOCK(vp);
 		do {
 			const struct bdevsw *bdev;
 
@@ -636,13 +637,10 @@ spec_open(void *v)
 			if ((name = bdevsw_getname(major(dev))) == NULL)
 break;
 
-			VOP_UNLOCK(vp);
-
 /* Try to autoload device module */
 			(void) module_autoload(name, MODULE_CLASS_DRIVER);
-			
-			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 		} while (gen != module_gen);
+		vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 
 		break;
 

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:34 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Call bdev_open without the vnode lock.

There is no need for it to serialize opens, because they are already
serialized by sd_opencnt which for block devices is always either 0
or 1.

There's not obviously any other reason why the vnode lock should be
held across bdev_open, other than that it might be nice to avoid
dropping it if not necessary.  For character devices we always have
to drop the vnode lock because open might hang indefinitely, when
opening a tty, which is not allowed while holding the vnode lock.


To generate a diff of this commit:
cvs rdiff -u -r1.186 -r1.187 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:26 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Note lock order for vnode lock, device_lock, v_interlock.


To generate a diff of this commit:
cvs rdiff -u -r1.185 -r1.186 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/miscfs/specfs/spec_vnops.c
diff -u src/sys/miscfs/specfs/spec_vnops.c:1.185 src/sys/miscfs/specfs/spec_vnops.c:1.186
--- src/sys/miscfs/specfs/spec_vnops.c:1.185	Mon Mar 28 12:34:17 2022
+++ src/sys/miscfs/specfs/spec_vnops.c	Mon Mar 28 12:34:25 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: spec_vnops.c,v 1.185 2022/03/28 12:34:17 riastradh Exp $	*/
+/*	$NetBSD: spec_vnops.c,v 1.186 2022/03/28 12:34:25 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -58,7 +58,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.185 2022/03/28 12:34:17 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: spec_vnops.c,v 1.186 2022/03/28 12:34:25 riastradh Exp $");
 
 #include 
 #include 
@@ -85,6 +85,14 @@ __KERNEL_RCSID(0, "$NetBSD: spec_vnops.c
 #include 
 #include 
 
+/*
+ * Lock order:
+ *
+ *	vnode lock
+ *	-> device_lock
+ *	-> struct vnode::v_interlock
+ */
+
 /* symbolic sleep message strings for devices */
 const char	devopn[] = "devopn";
 const char	devio[] = "devio";

CVS commit: src/sys/miscfs/specfs

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:34:26 UTC 2022

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
specfs: Note lock order for vnode lock, device_lock, v_interlock.


To generate a diff of this commit:
cvs rdiff -u -r1.185 -r1.186 src/sys/miscfs/specfs/spec_vnops.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

re: CVS commit: src/sys/miscfs/specfs

[Paul Goyette]

On Thu, 8 Sep 2016, matthew green wrote:


"Paul Goyette" writes:

Module Name:src
Committed By:   pgoyette
Date:   Thu Sep  8 00:07:48 UTC 2016

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
if_config processing wants to auto-load modules named with an if_ prefix,
while specfc wants to auto-load modules without the prefix.  For modules
which can be loaded both ways (ie, if_tap and if_tun), provide a simple
conversion table for specfs so it can auto-load the if_ module.

This table should always be quite small, and the auto-load operation is
relatively infrequent, so the additional overhead of comparing names should
be tolerable.


would you mind reverting this and implementing the "dependant"
module model mlelstv proposed?

the above is a hack and doesn't scale or work if a new module
with the same "problem" is introduced, as it requires the base
kernel to be patched, where as a pair of modules can be added
much more easily.


Sure, I can do that.  Point well taken re requiring base kernel mods to 
add new table entries.


It will take me a couple of days to complete and test.  I've got a lot 
of issues dealing with my new machine...



+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

re: CVS commit: src/sys/miscfs/specfs

[matthew green]

"Paul Goyette" writes:
> Module Name:  src
> Committed By: pgoyette
> Date: Thu Sep  8 00:07:48 UTC 2016
> 
> Modified Files:
>   src/sys/miscfs/specfs: spec_vnops.c
> 
> Log Message:
> if_config processing wants to auto-load modules named with an if_ prefix,
> while specfc wants to auto-load modules without the prefix.  For modules
> which can be loaded both ways (ie, if_tap and if_tun), provide a simple
> conversion table for specfs so it can auto-load the if_ module.
> 
> This table should always be quite small, and the auto-load operation is
> relatively infrequent, so the additional overhead of comparing names should
> be tolerable.

would you mind reverting this and implementing the "dependant"
module model mlelstv proposed?

the above is a hack and doesn't scale or work if a new module
with the same "problem" is introduced, as it requires the base
kernel to be patched, where as a pair of modules can be added
much more easily.

thanks.


.mrg.

Re: CVS commit: src/sys/miscfs/specfs

[Christos Zoulas]

In article <2015135437.b205bf...@cvs.netbsd.org>,
Paul Goyette  wrote:
>-=-=-=-=-=-
>
>Module Name:   src
>Committed By:  pgoyette
>Date:  Tue Dec 22 23:54:37 UTC 2015
>
>Modified Files:
>   src/sys/miscfs/specfs: spec_vnops.c
>
>Log Message:
>If we attempt to autoload a driver module, make sure we return an error
>if it fails.  Otherwise we might end up calling a builtin-but-disabled
>driver module and that can generate all sorts of issues...

Is there a reason to override the error?

christos

Re: CVS commit: src/sys/miscfs/specfs

[Paul Goyette]

On Wed, 23 Dec 2015, Christos Zoulas wrote:


In article <2015135437.b205bf...@cvs.netbsd.org>,
Paul Goyette  wrote:

-=-=-=-=-=-

Module Name:src
Committed By:   pgoyette
Date:   Tue Dec 22 23:54:37 UTC 2015

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
If we attempt to autoload a driver module, make sure we return an error
if it fails.  Otherwise we might end up calling a builtin-but-disabled
driver module and that can generate all sorts of issues...


Is there a reason to override the error?


No.  And since the error is already being reported, the change was
unneeded.

The bug I was following is elsewhere, so this change was backed out.


+--+--++
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:  |
| (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+--+--++

Re: CVS commit: src/sys/miscfs/specfs

[J. Hannken-Illjes]

On 29 Jun 2015, at 18:47, David Holland dholland-sourcechan...@netbsd.org 
wrote:

 On Mon, Jun 29, 2015 at 12:25:49PM -0400, Christos Zoulas wrote:
 Module Name: src
 Committed By:christos
 Date:Mon Jun 29 16:25:49 UTC 2015
 
 Modified Files:
  src/sys/miscfs/specfs: spec_vnops.c
 
 Log Message:
 Revert previous, and explain why.
 
 Hrm, shouldn't it be vp?
 
 (as it is, it's using the uvm_object lock pointer as the key)

Yes, just changed the key to the address of vp-v_specdev which is
invariant over the lifetime of the vnode.

Coverity is quite cool ...

--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)

Re: CVS commit: src/sys/miscfs/specfs

[David Holland]

On Mon, Jun 29, 2015 at 12:25:49PM -0400, Christos Zoulas wrote:
  Module Name: src
  Committed By:christos
  Date:Mon Jun 29 16:25:49 UTC 2015
  
  Modified Files:
   src/sys/miscfs/specfs: spec_vnops.c
  
  Log Message:
  Revert previous, and explain why.

Hrm, shouldn't it be vp?

(as it is, it's using the uvm_object lock pointer as the key)

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/sys/miscfs/specfs

[Antti Kantee]

On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote:
 Module Name:  src
 Committed By: ahoka
 Date: Tue Apr 13 01:15:56 UTC 2010
 
 Modified Files:
   src/sys/miscfs/specfs: spec_vnops.c
 
 Log Message:
 Autoload modules with any class.
 
 This fixes autoloading of pf, zfs and possibly others.

What is wrong with making drivers MODULE_CLASS_DRIVER?  I think we
should limit autoload aiming for safety, not splatter it all over.

Eventually everything will of course be fixed by, *tadaa*, devfs.
Meanwhile, please back this out and if you don't think calling a driver
a driver is the right thing to do, have a discussion.

Re: CVS commit: src/sys/miscfs/specfs

[Andrew Doran]

On Tue, Apr 13, 2010 at 10:47:51AM +0300, Antti Kantee wrote:
 On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote:
  Module Name:src
  Committed By:   ahoka
  Date:   Tue Apr 13 01:15:56 UTC 2010
  
  Modified Files:
  src/sys/miscfs/specfs: spec_vnops.c
  
  Log Message:
  Autoload modules with any class.
  
  This fixes autoloading of pf, zfs and possibly others.
 
 What is wrong with making drivers MODULE_CLASS_DRIVER?  I think we
 should limit autoload aiming for safety, not splatter it all over.
 
 Eventually everything will of course be fixed by, *tadaa*, devfs.
 Meanwhile, please back this out and if you don't think calling a driver
 a driver is the right thing to do, have a discussion.

What's the problem with autoloading any class?  Is it just a cosmetic thing?
One doesn't have to be an anointed driver to have an entry in /dev.

(It implies that the modules and devsw entries inhabit the same namespace.)

Re: CVS commit: src/sys/miscfs/specfs

[Andrew Doran]

On Tue, Apr 13, 2010 at 03:27:11PM +0300, Antti Kantee wrote:
 On Tue Apr 13 2010 at 12:18:38 +, Andrew Doran wrote:
  On Tue, Apr 13, 2010 at 10:47:51AM +0300, Antti Kantee wrote:
   On Tue Apr 13 2010 at 01:15:56 +, Adam Hoka wrote:
Module Name:src
Committed By:   ahoka
Date:   Tue Apr 13 01:15:56 UTC 2010

Modified Files:
src/sys/miscfs/specfs: spec_vnops.c

Log Message:
Autoload modules with any class.

This fixes autoloading of pf, zfs and possibly others.
   
   What is wrong with making drivers MODULE_CLASS_DRIVER?  I think we
   should limit autoload aiming for safety, not splatter it all over.
   
   Eventually everything will of course be fixed by, *tadaa*, devfs.
   Meanwhile, please back this out and if you don't think calling a driver
   a driver is the right thing to do, have a discussion.
  
  What's the problem with autoloading any class?  Is it just a cosmetic thing?
 
 I already mentioned my opinion above: aim for safety.  Since there
 is currently no common place to manage the different names we have,
 I don't want to discover some day that e.g. some secmodel name matches
 a devsw name.

So the kernel of the problem is namespace collisions, correct?

Would you agree that's it's the kernel programmers responsibility
to avoid conflicts?

If so how about sprinkling a little process in order to make it harder to
screw up?

Re: CVS commit: src/sys/miscfs/specfs

[Antti Kantee]

On Tue Apr 13 2010 at 12:32:38 +, Andrew Doran wrote:
 So the kernel of the problem is namespace collisions, correct?

Mostly.  Though I still think it's not expected that opening a /dev
node will load e.g. an exec package or secmodel even if that's what some
programmer wants.

 Would you agree that's it's the kernel programmers responsibility
 to avoid conflicts?
 
 If so how about sprinkling a little process in order to make it harder to
 screw up?

I'd say computers do conflict detection better.

Re: CVS commit: src/sys/miscfs/specfs

[Andrew Doran]

On Tue, Apr 13, 2010 at 03:40:24PM +0300, Antti Kantee wrote:
 On Tue Apr 13 2010 at 12:32:38 +, Andrew Doran wrote:
  So the kernel of the problem is namespace collisions, correct?
 
 Mostly.  Though I still think it's not expected that opening a /dev
 node will load e.g. an exec package or secmodel even if that's what some
 programmer wants.
 
  Would you agree that's it's the kernel programmers responsibility
  to avoid conflicts?
  
  If so how about sprinkling a little process in order to make it harder to
  screw up?
 
 I'd say computers do conflict detection better.

So the result of our teeth-pulling so far is:

1. You are concerned about namespace conflicts.  I am too.
2. I would be happy to see these managed through documentation and
   a straightforward approval process for adding modules.
3. You suggest that it would be better for the computer to manage it.

Can you suggest an alternative mechanism that will (a) allow us to
autoload things that are not anointed device drivers and (b) satisfy
your concerns?

As an example of something that wants autoloading both as a file
system and a driver, see ZFS.

Re: CVS commit: src/sys/miscfs/specfs

[Antti Kantee]

On Tue Apr 13 2010 at 13:25:24 +, Andrew Doran wrote:
 So the result of our teeth-pulling so far is:
 
 1. You are concerned about namespace conflicts.  I am too.
 2. I would be happy to see these managed through documentation and
a straightforward approval process for adding modules.

There is an additional pitfall with this.  We don't control all modules
and cannot be sure potential 3rd parties use the same processes.

 3. You suggest that it would be better for the computer to manage it.

Maybe not manage it, but at least detect it and fail gracefully.

 Can you suggest an alternative mechanism that will (a) allow us to
 autoload things that are not anointed device drivers and (b) satisfy
 your concerns?

devfs ;)

IOW, Given that we don't know where we most likely are in 6 months,
I'm not too keen on trying to spend energy to solve this right now ...

 As an example of something that wants autoloading both as a file
 system and a driver, see ZFS.

... if zfs is the only use case, since IIUC it's broken anyway.

Meanwhile, if there is something else which is catastrophically broken
due to lack of holy christening, we can revisit this and see if just
flipping the switch and maybe writing a simple audit script to verify
no collisions is the path of least wailing (this would be close to 2).

Re: CVS commit: src/sys/miscfs/specfs

[Masao Uebayashi]

On Tue, Apr 13, 2010 at 10:47 PM, Antti Kantee po...@cs.hut.fi wrote:
 On Tue Apr 13 2010 at 13:25:24 +, Andrew Doran wrote:
 So the result of our teeth-pulling so far is:

 1. You are concerned about namespace conflicts.  I am too.
 2. I would be happy to see these managed through documentation and
    a straightforward approval process for adding modules.

 There is an additional pitfall with this.  We don't control all modules
 and cannot be sure potential 3rd parties use the same processes.

 3. You suggest that it would be better for the computer to manage it.

 Maybe not manage it, but at least detect it and fail gracefully.

 Can you suggest an alternative mechanism that will (a) allow us to
 autoload things that are not anointed device drivers and (b) satisfy
 your concerns?

 devfs ;)

 IOW, Given that we don't know where we most likely are in 6 months,
 I'm not too keen on trying to spend energy to solve this right now ...

 As an example of something that wants autoloading both as a file
 system and a driver, see ZFS.

 ... if zfs is the only use case, since IIUC it's broken anyway.

 Meanwhile, if there is something else which is catastrophically broken
 due to lack of holy christening, we can revisit this and see if just
 flipping the switch and maybe writing a simple audit script to verify
 no collisions is the path of least wailing (this would be close to 2).

I like module names to become longer and unambiguous by making them
match the relative paths in src/sys, like sys/ufs/ffs/ffs.kmod.  I
don't think of any reason why module (driver, filesystem, ...) names
need to be very short.

Masao