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

["Gary Guo" <gary@garyguo.net>]

On Tue Sep 29, 2026 at 5:48 AM BST, 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.

This part is protected -- we mark FileDescriptionReservation as `!Send` which
prevents it being moved to another task. We also take care to make sure that
the task returned `current!()` is not allowed to be used outside the current
task.

> 	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.

So at least for this part, I think checking for `current->files` would be a
sufficient protection for Rust code. If we don't want to add these checks to the
C implementation, I think adding these checks to Rust wrappers would be ideal
until we figure out how to check things statically.

Calling fget/get_unused_fd_flags is still broken code -- so we could still
`WARN` on them.

>
> 	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.

I think for the simpler case, it is possible to check it statically by declaring
some functions to be only callable from "syscall context" and define ->release
to be not of that context.

Best,
Gary