Plug a race between fd table teardown and several loops

To export information from fd tables we have several loops which do
this:

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

Before r367777, fdescfree() acquired the fd table 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 race with teardown of file descriptor tables.

Acquire the exclusive fdtable lock after releasing the final table
reference to provide a barrier synchronizing with these loops.

Reported by:	pho
Reviewed by:	kib (previous version), mjg
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D27513
This commit is contained in:
markj 2020-12-09 14:05:08 +00:00
parent 88201c2c75
commit fb0c576172

View File

@ -2463,6 +2463,13 @@ fdescfree_fds(struct thread *td, struct filedesc *fdp, bool needclose)
struct file *fp;
int i, lastfile;
KASSERT(refcount_load(&fdp->fd_refcnt) == 0,
("%s: fd table %p carries references", __func__, fdp));
/* Serialize with threads iterating over the table. */
FILEDESC_XLOCK(fdp);
FILEDESC_XUNLOCK(fdp);
lastfile = fdlastfile_single(fdp);
for (i = 0; i <= lastfile; i++) {
fde = &fdp->fd_ofiles[i];
@ -2536,6 +2543,11 @@ pdescfree(struct thread *td)
void
fdescfree_remapped(struct filedesc *fdp)
{
#ifdef INVARIANTS
/* fdescfree_fds() asserts that fd_refcnt == 0. */
if (!refcount_release(&fdp->fd_refcnt))
panic("%s: fd table %p has extra references", __func__, fdp);
#endif
fdescfree_fds(curthread, fdp, 0);
}