On Mon, Nov 24, 2025 at 09:42:09PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Nov 24, 2025 at 05:37:09PM +0000, Visa Hankala wrote:
> > On Mon, Nov 24, 2025 at 05:21:19AM +0000, Tim Leslie wrote:
> > > 
> > > All,
> > > 
> > > fd_checkclosed() is only used in two places, both as a best‑effort race detector rather than a strong synchronization point. In sys_fcntl()’s F_SETLK/F_SETLKW path, it is called after VOP_ADVLOCK() to see if close() or dup2() won the race and then issue a compensating F_UNLCK if so. In kqueue_kqfilter(), fd filters use it to check whether knote_fdclose() might have missed the knote while it was being attached.
> > > 
> > > In both cases, the caller already holds a reference on the struct file obtained earlier from fd_getfile(). This patch converts the check to a direct atomic grab and then removing the helper.​
> > > 
> > > There is no intended semantic change beyond avoiding an extra mutex acquisition around what is already documented as a racy snapshot in both call sites.​
> > 
> > If the check is done without the mutex, what will ensure proper
> > ordering when there is a concurrent close/dup call?
> > 
> 
> Concurrent thread will override fdp->fd_ofiles[fd] just after
> `fd_fplock' mutex release, but the old value will be used. Also, the
> compiler will not reorder conditions in 'if' block.
> 
> There is no order problems here. This diff looks ok by me.
>

Ooops, The diff is wrong, it is NOT ok mvs. There is no ordering problem
with concurrent close()/dup() but the use-after-free issue caused by
concurrent fdexpand(). It replaces the dynamically allocated `fd_ofiles'
with the new one. Unlocked 'fdp->fd_ofiles' load returns the address of
already released memory.

Sorry for noise.