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

[jhb]

Summary

• 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.

[jhb]

Typo in commit title: s/FIONIO/FIONBIO/

[kib]

sys/kern/sys_generic.c

798

If you put the 'out1' label right before line 814, and use goto out1, then this fsetfl_unlock() call can be eliminated.

[jhb]

sys/kern/sys_generic.c

798

Hmmm, that seems a bit confusing to read. I did consider though restructuring this switch a bit to avoid the out label entirely, e.g.:

switch (com) {
case FIONCLEX:
...
case FIOCLEX:
...
case FIONBIO:
case FIOASYNC:
    f_flag = ...;
    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) {
            /* set or clear flag */
        }
    }
    fsetfl_unlock(fp);
    break;
 default:
    error = fo_ioctl(fp, com, data, td->td_ucred, td);
    break;
 }

 switch (locked) {

[kib]

sys/kern/sys_generic.c

798

So we trade duplicate unlock to duplicate fo_ioctl, I think this is reasonable.
Also goto out in FIOCLEX etc become plain break.

[jhb]

Rework switch statement

[kib]

sys/kern/sys_generic.c

795–811

What is the purpose of this line?

[jhb]

sys/kern/sys_generic.c

795–811

Oh, I forgot to remove it.