Re: Lockup with -current via SSH

Allen Briggs <[email protected]> writes:

> On Tue, Feb 25, 2003 at 11:33:52AM +0900, enami tsugutomo wrote:
> > > Until...I log in remotely via ssh.  Enter username, enter password.  Solid 
> > > lockup.  No console output, nothing in /var/log.  Reset to recover.  
> > > Repeatable every time. 
> > Probably, this is locking against myself in unp_internalize.
> 
> I think it may be something different.

Hmm, yes, there may be another issue :-).

Anyway, simple locking in unp_internalize() is also wrong, since it
will lock same one, leave simple lock held on error or tries to lock
multiple simplelocks.  I think changes something like below is
necessary.

enami.

Index: uipc_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.57
diff -u -r1.57 uipc_usrreq.c
--- uipc_usrreq.c	23 Feb 2003 14:37:34 -0000	1.57
+++ uipc_usrreq.c	25 Feb 2003 04:13:20 -0000
@@ -921,7 +921,7 @@
 	struct cmsghdr *cm = mtod(control, struct cmsghdr *);
 	struct file **rp;
 	struct file *fp;
-	int i, fd, *fdp;
+	int i, *fdp;
 	int nfds;
 	u_int neededspace;
 
@@ -930,16 +930,8 @@
 	    cm->cmsg_len != control->m_len)
 		return (EINVAL);
 
-	/* Verify that the file descriptors are valid */
-	nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) / sizeof(int);
-	fdp = (int *)CMSG_DATA(cm);
-	for (i = 0; i < nfds; i++) {
-		fd = *fdp++;
-		if (fd_getfile(fdescp, fd) == NULL)
-			return (EBADF);
-	}
-
 	/* Make sure we have room for the struct file pointers */
+	nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) / sizeof(int);
  morespace:
 	neededspace = CMSG_SPACE(nfds * sizeof(struct file *)) -
 	    control->m_len;
@@ -964,6 +956,22 @@
 	cm->cmsg_len = CMSG_LEN(nfds * sizeof(struct file *));
 	control->m_len = CMSG_SPACE(nfds * sizeof(struct file *));
 
+	/* Verify that the file descriptors are valid */
+	fdp = (int *)CMSG_DATA(cm);
+	for (i = 0; i < nfds; i++, fdp++) {
+		fp = fd_getfile(fdescp, *fdp);
+		if (fp == NULL) {
+			while (i-- > 0) {
+				fp = fdescp->fd_ofiles[*--fdp];
+				KDASSERT(fp != NULL);
+				(void) closef(fp, NULL);
+			}
+			return (EBADF);
+		}
+		fp->f_count++;
+		FILE_USE(fp);
+	}
+
 	/*
 	 * Transform the file descriptors into struct file pointers, in
 	 * reverse order so that if pointers are bigger than ints, the
@@ -979,9 +987,9 @@
 			panic("unp_internalize: file already closed");
 #endif
 		*rp-- = fp;
-		fp->f_count++;
 		fp->f_msgcount++;
 		simple_unlock(&fp->f_slock);
+		FILE_UNUSE(fp, NULL);
 		unp_rights++;
 	}
 	return (0);