Revert previous commit. The vnode returned by ptm_vn_open() is open and

can not simply be vrele()-ed on error. The code currently depends on
closef() to do the cleanup.

Reported-by: syzbot+b0e18235e96adf81883d@syzkaller.appspotmail.com

1 files changed, 28 insertions, 33 deletions

diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c
index 40b00e8ffb8..4c97ba84543 100644
--- a/sys/kern/tty_pty.c
+++ b/sys/kern/tty_pty.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tty_pty.c,v 1.106 2021/02/04 07:54:51 claudio Exp $	*/
+/*	$OpenBSD: tty_pty.c,v 1.107 2021/02/04 13:32:33 claudio Exp $	*/
 /*	$NetBSD: tty_pty.c,v 1.33.4.1 1996/06/02 09:08:11 mrg Exp $	*/
 
 /*
@@ -1093,7 +1093,6 @@ ptmioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
 	struct nameidata cnd, snd;
 	struct filedesc *fdp = p->p_fd;
 	struct file *cfp = NULL, *sfp = NULL;
-	struct vnode *cvp = NULL, *svp = NULL;
 	int cindx, sindx;
 	uid_t uid;
 	gid_t gid;
@@ -1103,11 +1102,24 @@ ptmioctl(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
 
 	switch (cmd) {
 	case PTMGET:
+		fdplock(fdp);
+		/* Grab two filedescriptors. */
+		if ((error = falloc(p, &cfp, &cindx)) != 0) {
+			fdpunlock(fdp);
+			break;
+		}
+		if ((error = falloc(p, &sfp, &sindx)) != 0) {
+			fdremove(fdp, cindx);
+			closef(cfp, p);
+			fdpunlock(fdp);
+			break;
+		}
+
 retry:
 		/* Find and open a free master pty. */
 		newdev = pty_getfree();
 		if ((error = check_pty(newdev)))
-			return (error);
+			goto bad;
 		pti = pt_softc[minor(newdev)];
 		NDINIT(&cnd, LOOKUP, NOFOLLOW|LOCKLEAF, UIO_SYSSPACE,
 		    pti->pty_pn, p);
@@ -1119,10 +1131,13 @@ retry:
 			 */
 			if (error == EIO && !pty_isfree(minor(newdev)))
 				goto retry;
-			return (error);
+			goto bad;
 		}
-		cvp = cnd.ni_vp;
-		VOP_UNLOCK(cvp);
+		cfp->f_flag = FREAD|FWRITE;
+		cfp->f_type = DTYPE_VNODE;
+		cfp->f_ops = &vnops;
+		cfp->f_data = (caddr_t) cnd.ni_vp;
+		VOP_UNLOCK(cnd.ni_vp);
 
 		/*
 		 * Open the slave.
@@ -1175,31 +1190,11 @@ retry:
 		/* now open it */
 		if ((error = ptm_vn_open(&snd)) != 0)
 			goto bad;
-		svp = snd.ni_vp;
-		VOP_UNLOCK(svp);
-
-		fdplock(fdp);
-		/* Grab two filedescriptors. */
-		if ((error = falloc(p, &cfp, &cindx)) != 0) {
-			fdpunlock(fdp);
-			goto bad;
-		}
-		if ((error = falloc(p, &sfp, &sindx)) != 0) {
-			fdremove(fdp, cindx);
-			closef(cfp, p);
-			fdpunlock(fdp);
-			goto bad;
-		}
-
-		cfp->f_flag = FREAD|FWRITE;
-		cfp->f_type = DTYPE_VNODE;
-		cfp->f_ops = &vnops;
-		cfp->f_data = (caddr_t)cvp;
-
 		sfp->f_flag = FREAD|FWRITE;
 		sfp->f_type = DTYPE_VNODE;
 		sfp->f_ops = &vnops;
-		sfp->f_data = (caddr_t)svp;
+		sfp->f_data = (caddr_t) snd.ni_vp;
+		VOP_UNLOCK(snd.ni_vp);
 
 		/* now, put the indexen and names into struct ptmget */
 		ptm->cfd = cindx;
@@ -1219,11 +1214,11 @@ retry:
 		break;
 	}
 	return (error);
-
 bad:
-	if (svp)
-		vrele(svp);
-	if (cvp)
-		vrele(cvp);
+	fdremove(fdp, cindx);
+	closef(cfp, p);
+	fdremove(fdp, sindx);
+	closef(sfp, p);
+	fdpunlock(fdp);
 	return (error);
 }