Re: [PATCH v2] rust: file: handle fd table teardown in file descriptor APIs

[Al Viro <viro@zeniv.linux.org.uk>]

On Tue, Sep 29, 2026 at 08:49:55AM +0000, Alice Ryhl wrote:

> So, I previously wrote some code that could invoke filp_close() to close
> a given fd ... from a workqueue. This was in the scenario where the
> process dies and the usual cleanup function gets called deferred from a
> workqueue instead of from the ioctl like usual.

Huh?

1) filp_close(file, NULL) doesn't do _anything_ to any descriptor tables;
the only requirements are that it should happen in _some_ thread context
(workqueue is fine) and that caller should not be holding any locks that
might be taken by ->flush() of the file in question (for a workqueue
callback it's fine as long as the callback itself is not holding any
of those).

2) any caller of filp_close(file, files_struct) must obviously guarantee
that files_struct won't be freed under it; passing current->files from
workqueue is safe in that respect, but obviously bogus.  Note that
descriptor table in question will *still* not be accessed; it serves
only as an opaque tag that identifies POSIX locks (and dnotify_struct
instances) related to the descriptor table in question.  IF you have
just manually removed the file in question from descriptor table
(file_close_fd()), you must call filp_close() passing it the same
descriptor table while that descriptor table is still guaranteed to
be alive.

Rationale is memory safety, actually - for POSIX locks descriptor table
serves as lock owner; the reference is opaque, but we don't want to have
it outlive freeing and reuse of the object it's pointing to.  So anything
that removes some file reference from a descriptor table is responsible
for corresponding filp_close() call done *before* the descriptor table
is gone.

Note that we only need to take care of the reference we remove from
descriptor table; files_struct destructor will call filp_close() for
anything still referenced from it.

Places where file reference is removed from the table:
	* do_close_on_exec(); filp_close() called in the same loop as
clearing the descriptor table slot.
	* do_dup2() in case the new slot had already been in use;
filp_close() called just before return.
	* file_close_fd_locked() callers. Three of those call filp_close()
as soon as they drop ->files_lock (close_fd(), __range_close() and
io_uring io_close()).  Remaining caller (close_fd_locked()) leaves that to
_its_ callers (binder_deferred_fd_close() and its equivalent Rust-side).

Again, normally both removal from descriptor table and filp_close() are
done by the same primitive...

> In this case the correct behavior was just to do nothing.

... leaking an opened file?  IDGI...

> It was a very easy mistake to make, and if such mistakes lead to null
> ptr derefs or worse, then I think it's worth doing something to reduce
> the bad consequences from this kind of mistake.