fd: Plug a race between fd table free and several loops

[markj]

Summary

To export info from an fd table we have several loops which do this:

FILDESC_SLOCK(fdp);
for (i = 0; fdp->fd_refcount > 0 && i < fdp->fd_lastfile; i++) {
	<export info for fd i>;
}
FILDESC_SUNLOCK(fdp);

Before r367777, fdescfree() acquired the fdtable exclusive lock between
decrementing fdp->fd_refcount and freeing table entries. This
serialized with the loop above, so the file at descriptor i would remain
valid until the lock is dropped. Now there is no serialization, so the
loops may access freed entries.

Restore the previous synchronization to fix the bug. This could be
micro-optimized to reduce the number of atomic operations but for now I
would just like to fix the bug.

Reported by: pho

[mjg]

I hacked up a short patch, but I'm not fond of it. I think this will be fine for now if you add an asssertion that fd_refcnt == 0

[markj]

Add an assertion.

[markj]

sys/kern/kern_descrip.c

2467 ↗

(On Diff #80445)

Actually this assertion isn't true for calls coming from fdescfree_remapped(). I think that code path can only be triggered by cloudabi.

[markj]

• Typos.
• Decrement fd_refcnt in fdescfree_remapped() if INVARIANTS is configured.

[cem]

I'm not sure I understand the solution, but totally believe there's a bug in this area.

sys/kern/kern_descrip.c

2467 ↗

(On Diff #80445)

Yeah, the _remapped paths are cloudabi only and should be removed with cloudabi.