PERFORCE change 83542 for review

[Robert Watson]

http://perforce.freebsd.org/chv.cgi?CH=83542

Change 83542 by rwatson at rwatson_peppercorn on 2005/09/13 15:11:21

	Trim down now (believed to be) unuxed fifo_ioctl() and
	fifo_kqfilter() VOP implementations, since they in theory are used
	only on open file descriptors, in which case the ioctls are via
	fifo_ioctl_f() and kqueue requests are via fifo_kqfilter_f().
	Generate warnings if they are entered for now.
	
	Annotate and re-implement fifo_ioctl_f(): don't arbitrarily
	forward ioctls to the socket layer, only forward the ones we
	explicitly support for fifos.  In the case of FIONREAD, don't
	forward the request to the write socket on a read-write fifo, or
	the read result is overwritten.  Annotate a nasty case for the
	undefined POSIX O_RDWR on fifos, in which failure of the second
	ioctl will result in the socket pair being in an inconsistent
	state.
	
	Assert copyright as I find myself rewriting non-trivial parts of
	fifofs.

Affected files ...

.. //depot/projects/netsmp/src/sys/fs/fifofs/fifo_vnops.c#13 edit

Differences ...

==== //depot/projects/netsmp/src/sys/fs/fifofs/fifo_vnops.c#13 (text+ko) ====

@@ -1,6 +1,8 @@
 /*-
  * Copyright (c) 1990, 1993, 1995
- *	The Regents of the University of California.  All rights reserved.
+ *	The Regents of the University of California.
+ * Copyright (c) 2005 Robert N. M. Watson
+ * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -296,7 +298,7 @@
 }
 
 /*
- * Device ioctl operation.
+ * Now unused vnode ioctl routine.
  */
 /* ARGSUSED */
 static int
@@ -310,35 +312,13 @@
 		struct thread *a_td;
 	} */ *ap;
 {
-	struct file filetmp;	/* Local, so need not be locked. */
-	int error;
 
-	if (ap->a_command == FIONBIO)
-		return (0);
-	if (ap->a_fflag & FREAD) {
-		filetmp.f_data = ap->a_vp->v_fifoinfo->fi_readsock;
-		filetmp.f_cred = ap->a_cred;
-		error = soo_ioctl(&filetmp, ap->a_command, ap->a_data,
-		    ap->a_td->td_ucred, ap->a_td);
-		if (error)
-			return (error);
-	}
-	if (ap->a_fflag & FWRITE) {
-		filetmp.f_data = ap->a_vp->v_fifoinfo->fi_writesock;
-		filetmp.f_cred = ap->a_cred;
-		error = soo_ioctl(&filetmp, ap->a_command, ap->a_data,
-		    ap->a_td->td_ucred, ap->a_td);
-		if (error)
-			return (error);
-	}
-	return (0);
+	printf("WARNING: fifo_ioctl called unexpectedly\n");
+	return (ENOTTY);
 }
 
 /*
- * Currently fifo_kqfilter() isn't reachable beause vop_kqfilter() is only
- * called for open files, in which case the fifo code has redirected the
- * caller to fifo_kqfilter_f() via the file descriptor operations vector.
- * This implementation should be garbage collected.
+ * Now unused vnode kqfilter routine.
  */
 /* ARGSUSED */
 static int
@@ -348,33 +328,9 @@
 		struct knote *a_kn;
 	} */ *ap;
 {
-	struct fifoinfo *fi = ap->a_vp->v_fifoinfo;
-	struct socket *so;
-	struct sockbuf *sb;
 
-	switch (ap->a_kn->kn_filter) {
-	case EVFILT_READ:
-		ap->a_kn->kn_fop = &fiforead_filtops;
-		so = fi->fi_readsock;
-		sb = &so->so_rcv;
-		break;
-	case EVFILT_WRITE:
-		ap->a_kn->kn_fop = &fifowrite_filtops;
-		so = fi->fi_writesock;
-		sb = &so->so_snd;
-		break;
-	default:
-		return (EINVAL);
-	}
-
-	ap->a_kn->kn_hook = (caddr_t)so;
-
-	SOCKBUF_LOCK(sb);
-	knlist_add(&sb->sb_sel.si_note, ap->a_kn, 1);
-	sb->sb_flags |= SB_KNOTE;
-	SOCKBUF_UNLOCK(sb);
-
-	return (0);
+	printf("WARNING: fifo_kqfilter called unexpectedly\n");
+	return (EINVAL);
 }
 
 static void
@@ -557,8 +513,25 @@
 	return (vnops.fo_close(fp, td));
 }
 
+/*
+ * The implementation of ioctl() for named fifos is complicated by the fact
+ * that we permit O_RDWR fifo file descriptors, meaning that the actions of
+ * ioctls may have to be applied to both the underlying sockets rather than
+ * just one.  The original implementation simply forward the ioctl to one
+ * or both sockets based on fp->f_flag.  We now consider each ioctl
+ * separately, as the composition effect requires careful ordering.
+ *
+ * We do not blindly pass all ioctls through to the socket in order to avoid
+ * providing unnecessary ioctls that might be improperly depended on by
+ * applications (such as socket-specific, routing, and interface ioctls).
+ *
+ * Unlike sys_pipe.c, fifos do not implement the deprecated TIOCSPGRP and
+ * TIOCGPGRP ioctls.  Earlier implementations of fifos did forward SIOCSPGRP
+ * and SIOCGPGRP ioctls, so we might need to re-add those here.
+ */
 static int
-fifo_ioctl_f(struct file *fp, u_long com, void *data, struct ucred *cred, struct thread *td)
+fifo_ioctl_f(struct file *fp, u_long com, void *data, struct ucred *cred,
+    struct thread *td)
 {
 	struct fifoinfo *fi;
 	struct file filetmp;	/* Local, so need not be locked. */
@@ -566,21 +539,58 @@
 
 	error = ENOTTY;
 	fi = fp->f_data;
-	if (com == FIONBIO)
+
+	switch (com) {
+	case FIONBIO:
+		/*
+		 * Non-blocking I/O is implemented at the fifo layer using
+		 * MSG_NBIO, so does not need to be forwarded down the stack.
+		 */
 		return (0);
-	if (fp->f_flag & FREAD) {
+
+	case FIOASYNC:
+	case FIOSETOWN:
+	case FIOGETOWN:
+		/*
+		 * These socket ioctls don't have any ordering requirements,
+		 * so are called in an arbitrary order, and only on the
+		 * sockets indicated by the file descriptor rights.
+		 *
+		 * XXXRW: If O_RDWR and the read socket accepts an ioctl but
+		 * the write socket doesn't, the socketpair is left in an
+		 * inconsistent state.
+		 */
+		if (fp->f_flag & FREAD) {
+			filetmp.f_data = fi->fi_readsock;
+			filetmp.f_cred = cred;
+			error = soo_ioctl(&filetmp, com, data, cred, td);
+			if (error)
+				return (error);
+		}
+		if (fp->f_flag & FWRITE) {
+			filetmp.f_data = fi->fi_writesock;
+			filetmp.f_cred = cred;
+			error = soo_ioctl(&filetmp, com, data, cred, td);
+		}
+		return (error);
+
+	case FIONREAD:
+		/*
+		 * FIONREAD will return 0 for non-readable descriptors, and
+		 * the results of FIONREAD on the read socket for readable
+		 * descriptors.
+		 */
+		if (!(fp->f_flag & FREAD)) {
+			*(int *)data = 0;
+			return (0);
+		}
 		filetmp.f_data = fi->fi_readsock;
 		filetmp.f_cred = cred;
-		error = soo_ioctl(&filetmp, com, data, cred, td);
-		if (error)
-			return (error);
-	}
-	if (fp->f_flag & FWRITE) {
-		filetmp.f_data = fi->fi_writesock;
-		filetmp.f_cred = cred;
-		error = soo_ioctl(&filetmp, com, data, cred, td);
+		return (soo_ioctl(&filetmp, com, data, cred, td));
+
+	default:
+		return (ENOTTY);
 	}
-	return (error);
 }
 
 /*