14898 port_associate PORT_SOURCE_FILE doesn't update user obj properly

Reviewed by: Nahum Shalman <nahamu+illumos@gmail.com>
Reviewed by: Andy Fiddaman <illumos@fiddaman.net>
Approved by: Richard Lowe <richlowe@richlowe.net>

6 files changed, 341 insertions, 12 deletions

diff --git a/usr/src/pkg/manifests/system-test-ostest.p5m b/usr/src/pkg/manifests/system-test-ostest.p5m
index a235dff463..68de4e2e33 100644
--- a/usr/src/pkg/manifests/system-test-ostest.p5m
+++ b/usr/src/pkg/manifests/system-test-ostest.p5m
@@ -96,6 +96,9 @@ file path=opt/os-tests/tests/pf_key/eacq-enabler mode=0555
 file path=opt/os-tests/tests/pf_key/kmc-update mode=0555
 file path=opt/os-tests/tests/pf_key/kmc-updater mode=0555
 file path=opt/os-tests/tests/poll_test mode=0555
+dir  path=opt/os-tests/tests/portfs
+file path=opt/os-tests/tests/portfs/file_assoc.32 mode=0555
+file path=opt/os-tests/tests/portfs/file_assoc.64 mode=0555
 dir  path=opt/os-tests/tests/sdevfs
 file path=opt/os-tests/tests/sdevfs/sdevfs_eisdir mode=0555
 dir  path=opt/os-tests/tests/secflags
diff --git a/usr/src/test/os-tests/runfiles/default.run b/usr/src/test/os-tests/runfiles/default.run
index c96c159c30..0cdb1352d9 100644
--- a/usr/src/test/os-tests/runfiles/default.run
+++ b/usr/src/test/os-tests/runfiles/default.run
@@ -136,3 +136,6 @@ pre = core_prereqs
 tests = ['coretests']
 
 [/opt/os-tests/tests/zen_umc_test]
+
+[/opt/os-tests/tests/portfs]
+tests = ['file_assoc.32', 'file_assoc.64']
diff --git a/usr/src/test/os-tests/tests/Makefile b/usr/src/test/os-tests/tests/Makefile
index 67e6bbfb69..76ec1f9a1b 100644
--- a/usr/src/test/os-tests/tests/Makefile
+++ b/usr/src/test/os-tests/tests/Makefile
@@ -28,6 +28,7 @@ SUBDIRS =       \
 		libtopo \
 		pf_key \
 		poll \
+		portfs \
 		sdevfs \
 		secflags \
 		sigqueue \
diff --git a/usr/src/test/os-tests/tests/portfs/Makefile b/usr/src/test/os-tests/tests/portfs/Makefile
new file mode 100644
index 0000000000..61d7d4bda4
--- /dev/null
+++ b/usr/src/test/os-tests/tests/portfs/Makefile
@@ -0,0 +1,61 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2022 Oxide Computer Company
+#
+
+PROGS = \
+	file_assoc
+
+PROGS32 = $(PROGS:%=%.32)
+PROGS64 = $(PROGS:%=%.64)
+
+ROOTOPTDIR = $(ROOT)/opt/os-tests/tests
+ROOTOPTPORTFS = $(ROOTOPTDIR)/portfs
+ROOTOPTPROGS = $(PROGS32:%=$(ROOTOPTPORTFS)/%) \
+	$(PROGS64:%=$(ROOTOPTPORTFS)/%)
+
+include $(SRC)/cmd/Makefile.cmd
+
+CSTD=$(GNU_C99)
+
+.KEEP_STATE:
+
+all: $(PROGS32) $(PROGS64)
+
+install: $(ROOTOPTPROGS)
+
+clean:
+
+$(ROOTOPTPROGS): $(PROGS32) $(PROGS64) $(ROOTOPTPORTFS)
+
+$(ROOTOPTDIR):
+	$(INS.dir)
+
+$(ROOTOPTPORTFS): $(ROOTOPTDIR)
+	$(INS.dir)
+
+$(ROOTOPTPORTFS)/%: %
+	$(INS.file)
+
+%.64: %.c
+	$(LINK64.c) -o $@ $< $(LDLIBS64)
+	$(POST_PROCESS)
+
+%.32: %.c
+	$(LINK.c) -o $@ $< $(LDLIBS)
+	$(POST_PROCESS)
+
+clobber:
+	$(RM) $(PROGS32) $(PROGS64)
+
+FRC:
diff --git a/usr/src/test/os-tests/tests/portfs/file_assoc.c b/usr/src/test/os-tests/tests/portfs/file_assoc.c
new file mode 100644
index 0000000000..6fa5c7de4e
--- /dev/null
+++ b/usr/src/test/os-tests/tests/portfs/file_assoc.c
@@ -0,0 +1,259 @@
+/*
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ */
+
+/*
+ * Copyright 2022 Oxide Computer Company
+ */
+
+/*
+ * This is designed to act as a basic test of PORT_SOURCE_FILE associations and
+ * a regression test for illumos#14898. In particular we want to verify certain
+ * behaviors of association and disassociation with respect to the value in the
+ * user payload. We will create and tear down the underlying event port each
+ * time. The rough cases are:
+ *
+ *   o associate, trigger, port_get -> first associate event
+ *   o associate, associate, trigger, port_get -> second associate event
+ *   o associate, trigger, associate, port_get -> second associate event
+ *   o associate, disassociate, port_get -> no event
+ *   o associate, trigger, disassociate, port_get -> no event
+ *   o associate, trigger, disassociate, associate, port_get -> second associate
+ *     event
+ *   o associate, trigger, disassociate, fstat, associate, port_get -> no event
+ */
+
+#include <port.h>
+#include <err.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <strings.h>
+#include <stdbool.h>
+#include <sys/sysmacros.h>
+
+static int fa_nfail = 0;
+static uintptr_t fa_user = 1;
+static char *fa_path;
+
+/*
+ * This is a series of actions that we want to be able to take on our port. We
+ * keep going until we do encounter a FA_DONE, at which point we do a
+ * port_get() to compare things.
+ */
+typedef enum {
+	FA_DONE,
+	FA_ASSOC,
+	FA_DEASSOC,
+	FA_FSTAT,
+	FA_TRIGGER
+} fa_act_t;
+
+#define	FA_MAX_EVENTS	6
+
+typedef struct {
+	bool fa_getevent;
+	const char *fa_msg;
+	fa_act_t fa_acts[FA_MAX_EVENTS];
+} fa_test_t;
+
+fa_test_t fa_tests[] = {
+	{ false, "port_get -> no event",
+	    { FA_TRIGGER, FA_DONE } },
+	{ false, "associate, port_get -> no event",
+	    { FA_ASSOC, FA_DONE } },
+	{ true, "associate, trigger, port_get -> first user",
+	    { FA_ASSOC, FA_TRIGGER, FA_DONE } },
+	{ true, "associate, associate, trigger, port_get -> second user",
+	    { FA_ASSOC, FA_ASSOC, FA_TRIGGER, FA_DONE } },
+	{ true, "associate, trigger, associate, port_get -> second user",
+	    { FA_ASSOC, FA_TRIGGER, FA_ASSOC, FA_DONE } },
+	{ false, "associate, disassociate, port_get -> no event",
+	    { FA_ASSOC, FA_DEASSOC, FA_DONE } },
+	{ false, "associate, trigger, disassociate, port_get -> no event",
+	    { FA_ASSOC, FA_TRIGGER, FA_DEASSOC, FA_DONE } },
+	{ true, "associate, trigger, disassociate, associate, port_get -> "
+	    "second user", { FA_ASSOC, FA_TRIGGER, FA_DEASSOC, FA_ASSOC,
+	    FA_DONE } },
+	{ false, "associate, trigger, disassociate, fstat, associate, port_get "
+	    "-> no event", { FA_ASSOC, FA_TRIGGER, FA_DEASSOC, FA_FSTAT,
+	    FA_ASSOC, FA_DONE } },
+};
+
+static void
+fa_run_test(int portfd, int filefd, fa_test_t *test)
+{
+	int ret;
+	uint_t nget;
+	struct stat st;
+	struct file_obj fo;
+	port_event_t pe;
+	struct timespec to;
+	bool pass;
+
+	/*
+	 * At the beginning of a test we stat our underlying file so we can make
+	 * sure our information is up to date. We purposefully keep it the same
+	 * across a run so that way certain tests will automatically trigger an
+	 * event on association.
+	 */
+	if (fstat(filefd, &st) != 0) {
+		warn("failed to stat %s", fa_path);
+		(void) printf("TEST FAILED: %s\n", test->fa_msg);
+		fa_nfail = 1;
+		return;
+	}
+
+	bzero(&fo, sizeof (fo));
+
+	for (uint_t i = 0; test->fa_acts[i] != FA_DONE; i++) {
+		uint32_t data;
+
+		switch (test->fa_acts[i]) {
+		case FA_ASSOC:
+			bzero(&fo, sizeof (fo));
+			fo.fo_atime = st.st_atim;
+			fo.fo_mtime = st.st_mtim;
+			fo.fo_ctime = st.st_ctim;
+			fo.fo_name = fa_path;
+
+			fa_user++;
+			if (port_associate(portfd, PORT_SOURCE_FILE,
+			    (uintptr_t)&fo, FILE_MODIFIED, (void *)fa_user) <
+			    0) {
+				warn("failed to associate event");
+				fa_nfail = 1;
+			}
+			break;
+		case FA_DEASSOC:
+			if (port_dissociate(portfd, PORT_SOURCE_FILE,
+			    (uintptr_t)&fo) != 0) {
+				warn("failed to dissociate event");
+				fa_nfail = 1;
+			}
+			break;
+		case FA_FSTAT:
+			if (fstat(filefd, &st) != 0) {
+				warn("failed to stat %s", fa_path);
+				fa_nfail = 1;
+			}
+			break;
+		case FA_TRIGGER:
+			data = arc4random();
+			if (write(filefd, &data, sizeof (data)) < 0) {
+				warn("failed to write data to %s", fa_path);
+			}
+			break;
+		default:
+			abort();
+		}
+	}
+
+	/*
+	 * At this point we attempt to see if there's an event for us. We
+	 * explicitly zero the timeout so we don't wait at all.
+	 */
+	bzero(&to, sizeof (to));
+	bzero(&pe, sizeof (pe));
+	nget = 1;
+	ret = port_getn(portfd, &pe, 1, &nget, &to);
+	if (ret < 0) {
+		warn("port_getn failed unexpectedly");
+		(void) printf("TEST FAILED: %s\n", test->fa_msg);
+		fa_nfail = 1;
+		return;
+	}
+
+	if (!test->fa_getevent) {
+		if (nget != 0) {
+			warnx("port_getn() returned an event, but we expected "
+			    "none");
+			(void) printf("portev_events: 0x%x, portev_source: "
+			    "0x%x\n", pe.portev_events, pe.portev_source);
+			(void) printf("TEST FAILED: %s\n", test->fa_msg);
+			fa_nfail = 1;
+		} else {
+			(void) printf("TEST PASSED: %s\n", test->fa_msg);
+		}
+		return;
+	} else {
+		if (nget == 0) {
+			warnx("port_getn() returned no events, but we expected "
+			    "one");
+			(void) printf("TEST FAILED: %s\n", test->fa_msg);
+			fa_nfail = 1;
+			return;
+		}
+	}
+
+	pass = true;
+	if (pe.portev_source != PORT_SOURCE_FILE) {
+		(void) printf("port source mismatch: found 0x%x, expected "
+		    "0x%x\n", pe.portev_source, PORT_SOURCE_FILE);
+		pass = false;
+	}
+
+	if (pe.portev_events != FILE_MODIFIED) {
+		(void) printf("port events mismatch: found 0x%x, expected "
+		    "0x%x\n", pe.portev_events, FILE_MODIFIED);
+		pass = false;
+	}
+
+	if ((uintptr_t)pe.portev_user != fa_user) {
+		(void) printf("port user mismatch: found 0x%p, expected "
+		    "0x%lx\n", pe.portev_user, fa_user);
+		pass = false;
+
+	}
+
+	if (pass) {
+		(void) printf("TEST PASSED: %s\n", test->fa_msg);
+	} else {
+		fa_nfail = 1;
+		(void) printf("TEST FAILED: %s\n", test->fa_msg);
+	}
+}
+
+int
+main(void)
+{
+	int fd;
+
+
+	if (asprintf(&fa_path, "/tmp/file_assoc_test.%d", getpid()) < 0) {
+		err(EXIT_FAILURE, "failed to create temp file");
+	}
+
+	fd = open(fa_path, O_RDWR | O_CREAT, 0644);
+	if (fd < 0) {
+		err(EXIT_FAILURE, "failed to create %s", fa_path);
+	}
+
+	/*
+	 * We open and close the underlying port that we're using for each run
+	 * to make sure that any associations that were created do not persist.
+	 */
+	for (uint_t i = 0; i < ARRAY_SIZE(fa_tests); i++) {
+		int port = port_create();
+		if (port < 0) {
+			err(EXIT_FAILURE, "failed to create event port");
+		}
+		fa_run_test(port, fd, &fa_tests[i]);
+		(void) close(port);
+	}
+
+	(void) close(fd);
+	(void) unlink(fa_path);
+	return (fa_nfail);
+}
diff --git a/usr/src/uts/common/fs/portfs/port_fop.c b/usr/src/uts/common/fs/portfs/port_fop.c
index 019de0540a..c9c417fda8 100644
--- a/usr/src/uts/common/fs/portfs/port_fop.c
+++ b/usr/src/uts/common/fs/portfs/port_fop.c
@@ -25,6 +25,7 @@
 
 /*
  * Copyright (c) 2018, Joyent, Inc.
+ * Copyright 2022 Oxide Computer Company
  */
 
 /*
@@ -257,7 +258,7 @@ const fs_operation_def_t	port_vnodesrc_template[] = {
 	VOPNAME_READ,		{ .femop_read = port_fop_read },
 	VOPNAME_WRITE,		{ .femop_write = port_fop_write },
 	VOPNAME_MAP,		{ .femop_map = port_fop_map },
-	VOPNAME_SETATTR, 	{ .femop_setattr = port_fop_setattr },
+	VOPNAME_SETATTR,	{ .femop_setattr = port_fop_setattr },
 	VOPNAME_CREATE,		{ .femop_create = port_fop_create },
 	VOPNAME_REMOVE,		{ .femop_remove = port_fop_remove },
 	VOPNAME_LINK,		{ .femop_link = port_fop_link },
@@ -266,7 +267,7 @@ const fs_operation_def_t	port_vnodesrc_template[] = {
 	VOPNAME_RMDIR,		{ .femop_rmdir = port_fop_rmdir },
 	VOPNAME_READDIR,	{ .femop_readdir = port_fop_readdir },
 	VOPNAME_SYMLINK,	{ .femop_symlink = port_fop_symlink },
-	VOPNAME_SETSECATTR, 	{ .femop_setsecattr = port_fop_setsecattr },
+	VOPNAME_SETSECATTR,	{ .femop_setsecattr = port_fop_setsecattr },
 	VOPNAME_VNEVENT,	{ .femop_vnevent = port_fop_vnevent },
 	NULL,	NULL
 };
@@ -275,7 +276,7 @@ const fs_operation_def_t	port_vnodesrc_template[] = {
  * Fsem - vfs ops hooks
  */
 const fs_operation_def_t	port_vfssrc_template[] = {
-	VFSNAME_UNMOUNT, 	{ .fsemop_unmount = port_fop_unmount },
+	VFSNAME_UNMOUNT,	{ .fsemop_unmount = port_fop_unmount },
 	NULL,	NULL
 };
 
@@ -730,8 +731,8 @@ port_cache_lookup_fop(portfop_cache_t *pfcp, pid_t pid, uintptr_t obj)
  * the vnode(s).
  */
 int
-port_fop_getdvp(void *objptr, vnode_t **vp, vnode_t **dvp,
-	char **cname, int *len, int follow)
+port_fop_getdvp(void *objptr, vnode_t **vp, vnode_t **dvp, char **cname,
+    int *len, int follow)
 {
 	int error = 0;
 	struct pathname pn;
@@ -815,7 +816,7 @@ port_getsrc(port_t *pp, int source)
  */
 static void
 port_check_timestamp(portfop_cache_t *pfcp, vnode_t *vp, vnode_t *dvp,
-	portfop_t *pfp, void *objptr, uintptr_t object)
+    portfop_t *pfp, void *objptr, uintptr_t object)
 {
 	vattr_t		vatt;
 	portfop_vp_t	*pvp = vp->v_fopdata;
@@ -1102,8 +1103,8 @@ port_install_fopdata(vnode_t *vp)
  */
 int
 port_pfp_setup(portfop_t **pfpp, port_t *pp, vnode_t *vp, portfop_cache_t *pfcp,
-	uintptr_t object, int events, void *user, char *cname, int clen,
-	vnode_t *dvp)
+    uintptr_t object, int events, void *user, char *cname, int clen,
+    vnode_t *dvp)
 {
 	portfop_t	*pfp = NULL;
 	port_kevent_t	*pkevp;
@@ -1465,16 +1466,17 @@ port_associate_fop(port_t *pp, int source, uintptr_t object, int events,
 		mutex_enter(&pvp->pvp_mutex);
 
 		/*
-		 * remove any queued up event.
+		 * Remove any queued up event.
 		 */
 		if (port_remove_done_event(pfp->pfop_pev)) {
 			pfp->pfop_flags &= ~PORT_FOP_KEV_ONQ;
 		}
 
 		/*
-		 * set new events to watch.
+		 * Set new events to watch and update the user pointer.
 		 */
 		pfp->pfop_events = events;
+		pfp->pfop_pev->portkev_user = user;
 
 		/*
 		 * If not active, mark it active even if it is being
@@ -1610,7 +1612,7 @@ port_close_fop(void *arg, int port, pid_t pid, int lastclose)
 	portfop_t	*pfpnext;
 	int		index, i;
 	port_source_t	*pse;
-	vnode_t 	*tdvp = NULL;
+	vnode_t		*tdvp = NULL;
 	vnode_t		*vpl[PORTFOP_NVP];
 
 	pse = port_getsrc(pp, PORT_SOURCE_FILE);
@@ -1980,7 +1982,7 @@ port_fop(vnode_t *vp, int op, int retval)
 		event  |= FILE_TRUNC;
 	}
 	if (event) {
-		port_fop_sendevent(vp, 	event, NULL, NULL);
+		port_fop_sendevent(vp, event, NULL, NULL);
 	}
 }