Re: CVS commit: src/share/man/man4

[Paul Goyette <paul%whooppee.com@localhost>]

On Tue, 5 Jan 2016, Christos Zoulas wrote:

> | Unless I can reliably determine which fd the program is using for its
> | access to /dev/filemon I don't have anything to which I can compare the
> | requested activity_log fd.
>
> You could scan the whole fd array and look for DT_MISC with fops ==
> filemon ops and if you find one that would cause a deadlock, deny.
> Again this is a hack... But at least it should prevent the deadlock.

Yeah, this is workable, even if it is a HACK !  :)

The attached patch borrows extensively from fd_free() routine in 
kern/kern_descrip.c

Let me know if this looks "good enough" and I will commit it.  (I'll 
also update the BUGS section of the man-page to describe the hack.)

FWIW, I have tested with log-file fd values of 1 and 4, while the 
/dev/filemon fd is always 3.  In the "1" case the patch correctly 
returned EINVAL (it would cause a deadlock), while the "4" case 
succeeded and no deadlock occurred.



+------------------+--------------------------+------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:      |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+

Index: filemon.c
===================================================================
RCS file: /cvsroot/src/sys/dev/filemon/filemon.c,v
retrieving revision 1.24
diff -u -p -r1.24 filemon.c
--- filemon.c	5 Jan 2016 22:08:54 -0000	1.24
+++ filemon.c	6 Jan 2016 05:05:04 -0000
@@ -279,8 +279,12 @@ static int
 filemon_ioctl(struct file * fp, u_long cmd, void *data)
 {
 	int error = 0;
+	int i, nf, fd;
 	struct filemon *filemon;
 	struct proc *tp;
+	fdfile_t *ff;
+	filedesc_t *fdp;
+	fdtab_t *dt;
 
 #ifdef DEBUG
 	log(logLevel, "filemon_ioctl(%lu)", cmd);;
@@ -303,8 +307,41 @@ filemon_ioctl(struct file * fp, u_long c
 		if (filemon->fm_fp)
 			fd_putfile(filemon->fm_fd);
 
+		/*
+		 * XXX HACK XXX
+		 *
+		 * Due to our taking an extra reference on the
+		 * descriptor's struct file, we need to ensure that
+		 * the descriptor number is at least as large as
+		 * the one used to access /dev/filemon.  Otherwise,
+		 * we get a deadlock during process exit, waiting
+		 * for the output file's reference count.
+		 */
+		fd = *((int *) data);
+		fdp = curproc->p_fd;
+		dt = curlwp->l_fd->fd_dt;
+		nf = dt->dt_nfiles;
+		error = EINVAL;
+		for (i = 0; i < nf; i++) {
+			if (i >= fd)
+				break;
+			ff = dt->dt_ff[i];
+			KASSERT(fd >= NDFDFILE ||
+				ff == (fdfile_t *)fdp->fd_dfdfile[i]);
+			if (ff == NULL)
+				continue;
+
+			if (ff->ff_file->f_type == DTYPE_MISC &&
+			    ff->ff_file->f_ops == &filemon_fileops) {
+				error = 0;
+				break;
+			}
+		}
+		if (error)
+			break;
+
 		/* Now set up the new one */
-		filemon->fm_fd = *((int *) data);
+		filemon->fm_fd = fd;
 		if ((filemon->fm_fp = fd_getfile(filemon->fm_fd)) == NULL) {
 			error = EBADF;
 			break;