kgess-mirror/freebsd-src
1
0
Fork 0
mirror of https://git.freebsd.org/src.git synced 2026-01-16 23:02:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

filedesc: Close race between fcntl(F_SETFL) and ioctl(FIONBIO/FIOASYNC)

Browse source 
- 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
This commit is contained in:
John Baldwin 2025-10-03 12:43:30 -04:00
parent 5c331f449e
commit dfd7d1610a
2 changed files with 48 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
sys/kern/sys_generic.c
View file

@ -729,7 +729,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);
@ -782,30 +782,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:

27
tests/sys/file/fcntlflags_test.c
View file

@ -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());
}