On Tue, Aug 26, 2025 at 11:23 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Mon, Aug 25, 2025 at 11:43:00PM +0200, Mateusz Guzik wrote:
> > __get_file_rcu() bails early:
> >
> > if (unlikely(!file_ref_get(&file->f_ref)))
> > return ERR_PTR(-EAGAIN);
> >
> > But get_file_rcu():
> > for (;;) {
> > struct file __rcu *file;
> >
> > file = __get_file_rcu(f);
> > if (!IS_ERR(file))
> > return file;
> > }
> >
> > So if this encounters a saturated refcount, the loop with never end.
> >
> > I don't know what makes the most sense to do here and I'm no position
> > to mess with any patches.
> >
> > This is not a serious problem either, so I would put this on the back
> > burner. Just reporting for interested.
>
> That's like past 2^63 - 1 references. Apart from an odd bug is that
> really something to worry about. I mean, we can add a VFS_WARN_ON_ONCE()
> in there of course but specifically handling that in the code doesn't
> seem sensible to me.
I'm not worried about the overflow, I am worried about the indefinite
& unkillable spin.
The only consumer of get_file_rcu() is get_mm_exe_file(), which is
ultimately used to serve out /proc/$pid/exe
Suppose there is a bug where the refcount got botched and and
file_ref_get() now fails.
With the current code the userspace processes who happen to try to
load the value will end up hanging from the user's pov as they will
keep spinning. No reaction to ^C or any other signal.
--
Mateusz Guzik <mjguzik gmail.com>
