Infinite loop in get_file_rcu() in face of a saturated ref count

Infinite loop in get_file_rcu() in face of a saturated ref count

[Mateusz Guzik]

__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.
-- 
Mateusz Guzik <mjguzik gmail.com>

Re: Infinite loop in get_file_rcu() in face of a saturated ref count

[Christian Brauner]

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.

Re: Infinite loop in get_file_rcu() in face of a saturated ref count

[Mateusz Guzik]

On Tue, Aug 26, 2025 at 11:23 AM Christian Brauner <brauner@kernel.org> 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>