Ensure that when a blockable open of fifo returns success, a valid

file descriptor opened for complimentary access exists as well.

The implementation of the guarantee is done by counting the
generations of readers and writers opens.  We return success and not
EINTR or ERESTART error, when the sleep for complimentary opening is
interrupted, but the generation was changed during the sleep.

Longer explanation: assume there are two threads, A doing open("fifo",
O_RDONLY) and B doing open("fifo", O_WRONLY), and no other threads
either trying to open the fifo, nor there are any file descriptors
referencing the fifo.  Before the change, it was possible e.g. for for
thread A to return a valid file descriptor, while thread B returned
EINTR if a signal to B was delivered simultaneously with the wakeup
from A.  After the change, in this situation both A::open() and
B::open() succeed and the signal is made "as if" it was noticed
slightly later.  Note that the signal actual delivery is not changed,
it is done by ast on syscall return path, so signal handler is still
executed before first instruction after syscall.

See PR for the code demonstrating the issue.

PR:	203162
Reported by:	Victor Stinner victor.stinner@gmail.com
Reviewed by:	jilles
Tested by:	bapt, pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week

1 files changed, 9 insertions, 2 deletions

diff --git a/sys/fs/fifofs/fifo_vnops.c b/sys/fs/fifofs/fifo_vnops.c
index e544bba..716faa3 100644
--- a/sys/fs/fifofs/fifo_vnops.c
+++ b/sys/fs/fifofs/fifo_vnops.c
@@ -64,6 +64,8 @@ struct fifoinfo {
 	struct pipe *fi_pipe;
 	long	fi_readers;
 	long	fi_writers;
+	u_int	fi_rgen;
+	u_int	fi_wgen;
 };
 
 static vop_print_t	fifo_print;
@@ -137,6 +139,7 @@ fifo_open(ap)
 	struct thread *td;
 	struct fifoinfo *fip;
 	struct pipe *fpipe;
+	u_int gen;
 	int error, stops_deferred;
 
 	vp = ap->a_vp;
@@ -164,6 +167,7 @@ fifo_open(ap)
 	PIPE_LOCK(fpipe);
 	if (ap->a_mode & FREAD) {
 		fip->fi_readers++;
+		fip->fi_rgen++;
 		if (fip->fi_readers == 1) {
 			fpipe->pipe_state &= ~PIPE_EOF;
 			if (fip->fi_writers > 0)
@@ -179,6 +183,7 @@ fifo_open(ap)
 			return (ENXIO);
 		}
 		fip->fi_writers++;
+		fip->fi_wgen++;
 		if (fip->fi_writers == 1) {
 			fpipe->pipe_state &= ~PIPE_EOF;
 			if (fip->fi_readers > 0)
@@ -187,6 +192,7 @@ fifo_open(ap)
 	}
 	if ((ap->a_mode & O_NONBLOCK) == 0) {
 		if ((ap->a_mode & FREAD) && fip->fi_writers == 0) {
+			gen = fip->fi_wgen;
 			VOP_UNLOCK(vp, 0);
 			stops_deferred = sigallowstop();
 			error = msleep(&fip->fi_readers, PIPE_MTX(fpipe),
@@ -194,7 +200,7 @@ fifo_open(ap)
 			if (stops_deferred)
 				sigdeferstop();
 			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-			if (error) {
+			if (error != 0 && gen == fip->fi_wgen) {
 				fip->fi_readers--;
 				if (fip->fi_readers == 0) {
 					PIPE_LOCK(fpipe);
@@ -214,6 +220,7 @@ fifo_open(ap)
 			 */
 		}
 		if ((ap->a_mode & FWRITE) && fip->fi_readers == 0) {
+			gen = fip->fi_rgen;
 			VOP_UNLOCK(vp, 0);
 			stops_deferred = sigallowstop();
 			error = msleep(&fip->fi_writers, PIPE_MTX(fpipe),
@@ -221,7 +228,7 @@ fifo_open(ap)
 			if (stops_deferred)
 				sigdeferstop();
 			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
-			if (error) {
+			if (error != 0 && gen == fip->fi_rgen) {
 				fip->fi_writers--;
 				if (fip->fi_writers == 0) {
 					PIPE_LOCK(fpipe);