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

[Alice Ryhl <aliceryhl@google.com>]

On Tue, Sep 29, 2026 at 05:48:43AM +0100, Al Viro wrote:
> On Fri, Sep 25, 2026 at 06:00:42PM +0200, Christian Brauner wrote:
> > On Thu, Sep 24, 2026 at 08:39:37AM +0000, Alice Ryhl wrote:
> 
> > > This looks like it should ideally be on the C side instead.
> > 
> > Where is this godforsaken broken code, that tries to fd_install() after
> > exit_files(). It is _a bug in the program_ that is not something the
> > apis need to work around.
> 
> 	More to the point, papering over that at runtime is wrong, and not
> just for modifying descriptor tables - fdget() is just as wrong in anything
> that can be called from tail of do_exit().
> 
> 	It's exactly the same as with "what if it gets called from an
> rcu callback?" - it's a bug, that's what.  Don't use these primitives
> in such context.
> 
> 	In particular, ->release() mentioned upthread should not be allowed
> to access _anything_ hanging off current, not just descriptor table.  Note
> that the last reference to an opened file might be sitting in an SCM_RIGHTS
> datagram pruned by AF_UNIX garbage collector; as far as the method is concerned,
> it might be called from random thread.
> 
> 	If it tries to access (let alone modify) the current descriptor table,
> you have no memory safety whatsoever and checking if current->files happens
> to be NULL is nowhere near enough to resolve that.
> 
> 	I don't know how to express that gracefully in terms of typechecking -
> sure, we could pass an empty token to each syscall, have fdget() et.al.
> require that as an argument and propagate the damn thing to all such callsites,
> but that would cause an insane amount of churn - if nothing else, ->ioctl()
> signature would have to be changed and there's a _lot_ of instances out there.
> And then there's the joy of dealing with ->sendmsg() and ->recvmsg(),
> thanks to SCM_RIGHTS datagrams, again (reading descriptor table on sendmsg()
> side, inserting into it on recvmsg()), especially when you consider the
> fact that ->sendmsg() and ->recvmsg() *are* callable from contexts where
> one shouldn't be allowed to access descriptor tables.  None of such
> call chains is going to trigger descriptor table access (e.g. knbd is
> not going to try and send SCM_RIGHTS datagrams, etc.), so it should be
> safe, but having compiler prove that without inflicting overhead on
> the code paths where it really wouldn't be welcome is not going to be trivial.

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.

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

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.

Alice