fcntl(F_SETFL): Don't unconditionally invoke FIONBIO and FIOASYNC

Currently, F_SETFL always invokes FIONBIO and FIOASYNC ioctls on the
file descriptor even if the state of the associated flag has not
changed.  This means that a character device driver that implements
non-blocking I/O but not async I/O needs a handler for FIOASYNC that
permits setting the value to 0.  This also means that
fcntl(fd, F_SETFL, fcntl(fd, F_GETFL)) can fail for a character device
driver that does not handle both FIONBIO and FIOASYNC.  These
requirements are not obvious nor well documented.

Instead, only invoke FIONBIO and FIOASYNC if the relevant flag changes
state.  This only requires a device driver to implement support for
FIONBIO or FIOASYNC if it supports the corresponding flag.

While here, if a request aims to toggle both F_NOBLOCK and F_ASYNC and
FIOASYNC fails, pass the previous state of F_NONBLOCK to FIONBIO
instead of always disabling non-blocking I/O and then possibly
reverting the flag back to on in f_flags.

Reviewed by:	mckusick, imp, kib, emaste
Differential Revision:	https://reviews.freebsd.org/D52403

1 files changed, 19 insertions, 13 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 057235574eb5..2a833d2eafbe 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -665,20 +665,26 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg)
 		} while (atomic_cmpset_int(&fp->f_flag, flg, tmp) == 0);
 		got_set = tmp & ~flg;
 		got_cleared = flg & ~tmp;
-		tmp = fp->f_flag & FNONBLOCK;
-		error = fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
-		if (error != 0)
-			goto revert_f_setfl;
-		tmp = fp->f_flag & FASYNC;
-		error = fo_ioctl(fp, FIOASYNC, &tmp, td->td_ucred, td);
-		if (error == 0) {
-			fdrop(fp, td);
-			break;
+		if (((got_set | got_cleared) & FNONBLOCK) != 0) {
+			tmp = fp->f_flag & FNONBLOCK;
+			error = fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
+			if (error != 0)
+				goto revert_flags;
+		}
+		if (((got_set | got_cleared) & FASYNC) != 0) {
+			tmp = fp->f_flag & FASYNC;
+			error = fo_ioctl(fp, FIOASYNC, &tmp, td->td_ucred, td);
+			if (error != 0)
+				goto revert_nonblock;
+		}
+		fdrop(fp, td);
+		break;
+revert_nonblock:
+		if (((got_set | got_cleared) & FNONBLOCK) != 0) {
+			tmp = ~fp->f_flag & FNONBLOCK;
+			(void)fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
 		}
-		atomic_clear_int(&fp->f_flag, FNONBLOCK);
-		tmp = 0;
-		(void)fo_ioctl(fp, FIONBIO, &tmp, td->td_ucred, td);
-revert_f_setfl:
+revert_flags:
 		do {
 			tmp = flg = fp->f_flag;
 			tmp &= ~FCNTLFLAGS;