--ikeVEW9yuYc//A+q
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Mar 05, 2005 at 11:07:26AM -0500, J Chapman Flack wrote:
> Martin Husemann asked:
> 
> > Can you try running it on a kernel with options LOCKDEBUG?
> 
> simple_lock: locking against myself
> lock: 0xcb752264, currently at: ../../../../kern/kern_descrip.c:597
> on CPU 1
> last locked: ../../../../kern/kern_descrip.c:212
> last unlocked: ../../../../kern/vfs_syscalls.c:1176
> 

Please try to apply attached patch. That should fix the locking
error in ibcs2_sys_ioctl().

Jaromir
-- 
Jaromir Dolecek <jdolecek@NetBSD.org>            http://www.NetBSD.cz/
-=- We can walk our road together if our goals are all the same;     -=-
-=- We can run alone and free if we pursue a different aim.          -=-

--ikeVEW9yuYc//A+q
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ibcs2ioctl.diff"

Index: ibcs2_ioctl.c
===================================================================
RCS file: /cvsroot/src/sys/compat/ibcs2/ibcs2_ioctl.c,v
retrieving revision 1.31
diff -u -p -r1.31 ibcs2_ioctl.c
--- ibcs2_ioctl.c	5 Nov 2003 04:03:43 -0000	1.31
+++ ibcs2_ioctl.c	5 Mar 2005 17:12:00 -0000
@@ -118,6 +118,7 @@ static void stios2btios __P((struct ibcs
 static void btios2stios __P((struct termios *, struct ibcs2_termios *));
 static void stios2stio __P((struct ibcs2_termios *, struct ibcs2_termio *));
 static void stio2stios __P((struct ibcs2_termio *, struct ibcs2_termios *));
+static int ibcs2_ioctlcmd __P((int));
 
 static void
 stios2btios(st, bt)
@@ -331,6 +332,31 @@ stio2stios(t, ts)
 	memcpy(ts->c_cc, t->c_cc, IBCS2_NCC);
 }
 
+static int
+ibcs2_ioctlcmd(cmd)
+	int cmd;
+{
+	switch (cmd) {
+	case IBCS2_TIOCGWINSZ:
+		cmd = TIOCGWINSZ;
+		break;
+
+	case IBCS2_TIOCSWINSZ:
+		cmd = TIOCSWINSZ;
+		break;
+
+	case IBCS2_I_NREAD:     /* STREAMS */
+		cmd = FIONREAD;
+		break;
+
+	default:
+		panic("ibcs2_ioctlcmd: unknown cmd '%d'", cmd);
+		/* NOTREACHED */
+	}
+
+	return cmd;
+}
+
 int
 ibcs2_sys_ioctl(l, v, retval)
 	struct lwp *l;
@@ -346,7 +372,7 @@ ibcs2_sys_ioctl(l, v, retval)
 	struct filedesc *fdp = p->p_fd;
 	struct file *fp;
 	int (*ctl)(struct file *, u_long, void *, struct proc *);
-	int error;
+	int error, locked = 1;
 
 	if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL) {
 		DPRINTF(("ibcs2_ioctl(%d): bad fd %d ", p->p_pid,
@@ -354,8 +380,11 @@ ibcs2_sys_ioctl(l, v, retval)
 		return EBADF;
 	}
 
+	FILE_USE(fp);
+
 	if ((fp->f_flag & (FREAD|FWRITE)) == 0) {
 		DPRINTF(("ibcs2_ioctl(%d): bad fp flag ", p->p_pid));
+		FILE_UNUSE(fp, p);
 		return EBADF;
 	}
 
@@ -371,7 +400,7 @@ ibcs2_sys_ioctl(l, v, retval)
 		struct ibcs2_termio st;
 	
 		if ((error = (*ctl)(fp, TIOCGETA, (caddr_t)&bts, p)) != 0)
-			return error;
+			break;
 	
 		btios2stios (&bts, &sts);
 		if (SCARG(uap, cmd) == IBCS2_TCGETA) {
@@ -381,11 +410,11 @@ ibcs2_sys_ioctl(l, v, retval)
 			if (error)
 				DPRINTF(("ibcs2_ioctl(%d): copyout failed ",
 					 p->p_pid));
-			return error;
-		} else
-			return copyout((caddr_t)&sts, SCARG(uap, data),
+		} else {
+			error = copyout((caddr_t)&sts, SCARG(uap, data),
 					sizeof (sts));
-		/*NOTREACHED*/
+		}
+		break;
 	    }
 
 	case IBCS2_TCSETA:
@@ -400,14 +429,14 @@ ibcs2_sys_ioctl(l, v, retval)
 				    sizeof(st))) != 0) {
 			DPRINTF(("ibcs2_ioctl(%d): TCSET copyin failed ",
 				 p->p_pid));
-			return error;
+			break;
 		}
 
 		/* get full BSD termios so we don't lose information */
 		if ((error = (*ctl)(fp, TIOCGETA, (caddr_t)&bts, p)) != 0) {
 			DPRINTF(("ibcs2_ioctl(%d): TCSET ctl failed fd %d ",
 				 p->p_pid, SCARG(uap, fd)));
-			return error;
+			break;
 		}
 
 		/*
@@ -418,8 +447,9 @@ ibcs2_sys_ioctl(l, v, retval)
 		stio2stios(&st, &sts);
 		stios2btios(&sts, &bts);
 
-		return (*ctl)(fp, SCARG(uap, cmd) - IBCS2_TCSETA + TIOCSETA,
+		error = (*ctl)(fp, SCARG(uap, cmd) - IBCS2_TCSETA + TIOCSETA,
 			      (caddr_t)&bts, p);
+		break;
 	    }
 
 	case IBCS2_XCSETA:
@@ -431,11 +461,12 @@ ibcs2_sys_ioctl(l, v, retval)
 
 		if ((error = copyin(SCARG(uap, data), (caddr_t)&sts,
 				    sizeof (sts))) != 0) {
-			return error;
+			break;
 		}
 		stios2btios (&sts, &bts);
-		return (*ctl)(fp, SCARG(uap, cmd) - IBCS2_XCSETA + TIOCSETA,
+		error = (*ctl)(fp, SCARG(uap, cmd) - IBCS2_XCSETA + TIOCSETA,
 			      (caddr_t)&bts, p);
+		break;
 	    }
 
 	case IBCS2_OXCSETA:
@@ -447,11 +478,12 @@ ibcs2_sys_ioctl(l, v, retval)
 
 		if ((error = copyin(SCARG(uap, data), (caddr_t)&sts,
 				    sizeof (sts))) != 0) {
-			return error;
+			break;
 		}
 		stios2btios (&sts, &bts);
-		return (*ctl)(fp, SCARG(uap, cmd) - IBCS2_OXCSETA + TIOCSETA,
+		error = (*ctl)(fp, SCARG(uap, cmd) - IBCS2_OXCSETA + TIOCSETA,
 			      (caddr_t)&bts, p);
+		break;
 	    }
 
 	case IBCS2_TCSBRK:
@@ -461,13 +493,14 @@ ibcs2_sys_ioctl(l, v, retval)
 		t = (t ? t : 1) * hz * 4;
 		t /= 10;
 		if ((error = (*ctl)(fp, TIOCSBRK, (caddr_t)0, p)))
-			return error;
+			break;
 		error = tsleep((caddr_t)&t, PZERO | PCATCH, "ibcs2_tcsbrk", t);
 		if (error == EINTR || error == ERESTART) {
 			(*ctl)(fp, TIOCCBRK, (caddr_t)0, p);
-			return EINTR;
+			error = EINTR;
 		} else
-			return (*ctl)(fp, TIOCCBRK, (caddr_t)0, p);
+			error = (*ctl)(fp, TIOCCBRK, (caddr_t)0, p);
+		break;
 	    }
 
 	case IBCS2_TCXONC:
@@ -476,14 +509,20 @@ ibcs2_sys_ioctl(l, v, retval)
 		case 0:
 		case 1:
 			DPRINTF(("ibcs2_ioctl(%d): TCXONC ", p->p_pid));
-			return ENOSYS;
+			error = ENOSYS;
+			break;
 		case 2:
-			return (*ctl)(fp, TIOCSTOP, (caddr_t)0, p);
+			error = (*ctl)(fp, TIOCSTOP, (caddr_t)0, p);
+			break;
 		case 3:
-			return (*ctl)(fp, TIOCSTART, (caddr_t)1, p);
+			error = (*ctl)(fp, TIOCSTART, (caddr_t)1, p);
+			break;
 		default:
-			return EINVAL;
+			error = EINVAL;
+			break;
 		}
+
+		break;
 	    }
 
 	case IBCS2_TCFLSH:
@@ -501,22 +540,17 @@ ibcs2_sys_ioctl(l, v, retval)
 			arg = FREAD | FWRITE;
 			break;
 		default:
-			return EINVAL;
+			error = EINVAL;
+			goto out;
 		}
-		return (*ctl)(fp, TIOCFLUSH, (caddr_t)&arg, p);
+		error = (*ctl)(fp, TIOCFLUSH, (caddr_t)&arg, p);
+		break;
 	    }
 
-	case IBCS2_TIOCGWINSZ:
-		SCARG(uap, cmd) = TIOCGWINSZ;
-		return sys_ioctl(l, uap, retval);
-
-	case IBCS2_TIOCSWINSZ:
-		SCARG(uap, cmd) = TIOCSWINSZ;
-		return sys_ioctl(l, uap, retval);
-
 	case IBCS2_TIOCGPGRP:
-		return copyout((caddr_t)&p->p_pgrp->pg_id, SCARG(uap, data),
+		error = copyout((caddr_t)&p->p_pgrp->pg_id, SCARG(uap, data),
 				sizeof(p->p_pgrp->pg_id));
+		break;
 
 	case IBCS2_TIOCSPGRP:	/* XXX - is uap->data a pointer to pgid? */
 	    {
@@ -524,19 +558,18 @@ ibcs2_sys_ioctl(l, v, retval)
 
 		SCARG(&sa, pid) = 0;
 		SCARG(&sa, pgid) = (int)SCARG(uap, data);
-		if ((error = sys_setpgid(l, &sa, retval)) != 0)
-			return error;
-		return 0;
+		error = sys_setpgid(l, &sa, retval);
+		break;
 	    }
 
 	case IBCS2_TCGETSC:	/* SCO console - get scancode flags */
-		return ENOSYS;
-
 	case IBCS2_TCSETSC:	/* SCO console - set scancode flags */
-		return ENOSYS;
+		error = ENOSYS;
+		break;
 
 	case IBCS2_SIOCSOCKSYS:
-		return ibcs2_socksys(l, uap, retval);
+		error = ibcs2_socksys(l, uap, retval);
+		break;
 
 	case IBCS2_FIONBIO:
 		{
@@ -544,20 +577,38 @@ ibcs2_sys_ioctl(l, v, retval)
 
 			if ((error = copyin(SCARG(uap, data), &arg,
 			    sizeof(arg))) != 0)
-				return error;
+				break;
 
-		    	return (*ctl)(fp, FIONBIO, (caddr_t)&arg, p);
+		    	error = (*ctl)(fp, FIONBIO, (caddr_t)&arg, p);
+			break;
 		}
 
+	case IBCS2_TIOCGWINSZ:
+	case IBCS2_TIOCSWINSZ:
 	case IBCS2_I_NREAD:     /* STREAMS */
-	        SCARG(uap, cmd) = FIONREAD;
-		return sys_ioctl(l, uap, retval);
+		SCARG(uap, cmd) = ibcs2_ioctlcmd(SCARG(uap, cmd));
+
+		/*
+		 * Need to unlock file descriptor before calling sys_ioctl(),
+		 * since that expects the descriptor unlocked and attemps
+		 * to lock it.
+		 */
+		FILE_UNUSE(fp, p);
+		locked = 0;
+		error = sys_ioctl(l, uap, retval);
+		break;
 
 	default:
 		DPRINTF(("ibcs2_ioctl(%d): unknown cmd 0x%x ",
 			 p->p_pid, SCARG(uap, cmd)));
-		return ENOSYS;
+		error = ENOSYS;
+		break;
 	}
+
+    out:
+	if (__predict_true(locked))
+		FILE_UNUSE(fp, p);
+	return error;
 }
 
 int

--ikeVEW9yuYc//A+q--