Subject: Re: Lockup with -current via SSH
To: Allen Briggs <briggs@wasabisystems.com>
From: enami tsugutomo <enami@sm.sony.co.jp>
List: current-users
Date: 02/25/2003 14:28:34
Allen Briggs <briggs@wasabisystems.com> 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);