From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
"Georgios Androutsopoulos" <georgeandrout13@gmail.com>,
"Miguel Ojeda" <ojeda@kernel.org>, "Jan Kara" <jack@suse.cz>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Tamir Duberstein" <tamird@kernel.org>,
"Alexandre Courbot" <acourbot@nvidia.com>,
"Onur Özkan" <work@onurozkan.dev>,
linux-fsdevel@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] rust: file: handle fd table teardown in file descriptor APIs
Date: Tue, 29 Sep 2026 05:48:43 +0100 [thread overview]
Message-ID: <20260929044843.GA3909609@ZenIV> (raw)
In-Reply-To: <20260925-stellen-brummen-festrede-266af0305ac7@brauner>
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.
Al, finally back to the state when reading from screen is tolerable for
reasonably long time - dry eyes were _really_ not fun to deal with...
next prev parent reply other threads:[~2026-09-29 4:48 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-23 2:23 Georgios Androutsopoulos
2026-09-24 8:39 ` Alice Ryhl
2026-09-24 16:06 ` Georgios Androutsopoulos
2026-09-24 16:18 ` Pedro Falcato
2026-09-25 16:00 ` Christian Brauner
2026-09-29 4:48 ` Al Viro [this message]
2026-09-29 8:49 ` Alice Ryhl
2026-09-29 13:28 ` Al Viro
2026-09-29 13:35 ` Alice Ryhl
2026-09-29 14:53 ` Al Viro
2026-09-29 12:24 ` Gary Guo
2026-09-29 13:51 ` Al Viro
2026-09-29 16:07 ` Gary Guo
2026-09-29 17:02 ` Al Viro
2026-09-29 18:22 ` Gary Guo
2026-09-29 19:47 ` Al Viro
2026-09-28 10:42 ` kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260929044843.GA3909609@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=brauner@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=georgeandrout13@gmail.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®