The branch stable/14 has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=6acf7356d859a59f237f14bfb5a28ded63480a5f
commit 6acf7356d859a59f237f14bfb5a28ded63480a5f Author: John Baldwin <[email protected]> AuthorDate: 2025-10-03 16:43:30 +0000 Commit: John Baldwin <[email protected]> CommitDate: 2025-12-18 17:01:56 +0000 filedesc: Close race between fcntl(F_SETFL) and ioctl(FIONBIO/FIOASYNC) - Use the recently-added fsetfl_lock/unlock API to synchronize direct FIONBIO and FIOASYNC ioctls with fcntl(F_SETFL). - While here, skip calling the underlying ioctl if the flag's current state matches the requested state. - Also while here, only update the flag state if the underlying ioctl succeeds. This fixes a bug where the flags represented the new state even if the underlying ioctl failed. A test is added for this last case that a failing FIOASYNC on /dev/null doesn't result in setting O_ASYNC in the file flags. Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D52721 (cherry picked from commit dfd7d1610aeb654aafcaa8cf77b75fc5c7d60250) --- sys/kern/sys_generic.c | 36 +++++++++++++++++++++--------------- tests/sys/file/fcntlflags_test.c | 27 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c index 6a80ddbe86ed..6590b1ac9c59 100644 --- a/sys/kern/sys_generic.c +++ b/sys/kern/sys_generic.c @@ -728,7 +728,7 @@ kern_ioctl(struct thread *td, int fd, u_long com, caddr_t data) { struct file *fp; struct filedesc *fdp; - int error, tmp, locked; + int error, f_flag, tmp, locked; AUDIT_ARG_FD(fd); AUDIT_ARG_CMD(com); @@ -781,30 +781,36 @@ kern_ioctl(struct thread *td, int fd, u_long com, caddr_t data) goto out; } + f_flag = 0; switch (com) { case FIONCLEX: fdp->fd_ofiles[fd].fde_flags &= ~UF_EXCLOSE; - goto out; + break; case FIOCLEX: fdp->fd_ofiles[fd].fde_flags |= UF_EXCLOSE; - goto out; - case FIONBIO: - if ((tmp = *(int *)data)) - atomic_set_int(&fp->f_flag, FNONBLOCK); - else - atomic_clear_int(&fp->f_flag, FNONBLOCK); - data = (void *)&tmp; break; + case FIONBIO: case FIOASYNC: - if ((tmp = *(int *)data)) - atomic_set_int(&fp->f_flag, FASYNC); - else - atomic_clear_int(&fp->f_flag, FASYNC); - data = (void *)&tmp; + f_flag = com == FIONBIO ? FNONBLOCK : FASYNC; + tmp = *(int *)data; + fsetfl_lock(fp); + if (((fp->f_flag & f_flag) != 0) != (tmp != 0)) { + error = fo_ioctl(fp, com, (void *)&tmp, td->td_ucred, + td); + if (error == 0) { + if (tmp != 0) + atomic_set_int(&fp->f_flag, f_flag); + else + atomic_clear_int(&fp->f_flag, f_flag); + } + } + fsetfl_unlock(fp); + break; + default: + error = fo_ioctl(fp, com, data, td->td_ucred, td); break; } - error = fo_ioctl(fp, com, data, td->td_ucred, td); out: switch (locked) { case LA_XLOCKED: diff --git a/tests/sys/file/fcntlflags_test.c b/tests/sys/file/fcntlflags_test.c index 009bb87371a1..15a18c113c4a 100644 --- a/tests/sys/file/fcntlflags_test.c +++ b/tests/sys/file/fcntlflags_test.c @@ -24,6 +24,7 @@ * SUCH DAMAGE. */ +#include <sys/filio.h> #include <errno.h> #include <fcntl.h> #include <stdio.h> @@ -95,12 +96,38 @@ ATF_TC_BODY(exec_only_sh, tc) basic_tests("/bin/sh", O_EXEC, "O_EXEC"); } +ATF_TC_WITHOUT_HEAD(fioasync_dev_null); +ATF_TC_BODY(fioasync_dev_null, tc) +{ + int fd, flags1, flags2, val; + + fd = open("/dev/null", O_RDONLY); + ATF_REQUIRE_MSG(fd != -1, "open(\"/dev/null\") failed: %s", + strerror(errno)); + + flags1 = fcntl(fd, F_GETFL); + ATF_REQUIRE_MSG(flags1 != -1, "fcntl(F_GETFL) (1) failed: %s", + strerror(errno)); + ATF_REQUIRE((flags1 & O_ASYNC) == 0); + + val = 1; + ATF_REQUIRE_ERRNO(EINVAL, ioctl(fd, FIOASYNC, &val) == -1); + + flags2 = fcntl(fd, F_GETFL); + ATF_REQUIRE_MSG(flags2 != -1, "fcntl(F_GETFL) (2) failed: %s", + strerror(errno)); + ATF_REQUIRE_INTEQ(flags1, flags2); + + (void)close(fd); +} + ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, read_only_null); ATF_TP_ADD_TC(tp, write_only_null); ATF_TP_ADD_TC(tp, read_write_null); ATF_TP_ADD_TC(tp, exec_only_sh); + ATF_TP_ADD_TC(tp, fioasync_dev_null); return (atf_no_error()); }
