Re: another use-after-free in ep_remove_wait_queue()

[Suren Baghdasaryan]

On Fri, Jan 20, 2023 at 9:18 PM Hillf Danton <hdanton@xxxxxxxx> wrote:
>
> On Fri, 20 Jan 2023 08:28:25 -0800 Suren Baghdasaryan <surenb@xxxxxxxxxx>
> > On Fri, Jan 20, 2023 at 1:00 AM Hillf Danton <hdanton@xxxxxxxx> wrote:
> > > +++ b/kernel/sched/psi.c
> > > @@ -1529,6 +1529,7 @@ static int psi_fop_release(struct inode
> > >  {
> > >         struct seq_file *seq = file->private_data;
> > >
> > > +       eventpoll_release_file(file);
> >
> > Be careful here and see the comment in
> > https://elixir.bootlin.com/linux/latest/source/fs/eventpoll.c#L912.
> > eventpoll_release_file() assumes that the last fput() was called and
> > nobody other than ep_free() will race with us. So, this will not be
> > that simple.
>
> The epmutex serializes eventpoll_release_file() and ep_free(). And this
> is in psi_fop_release(), so no chance is likely left for another release.
>
> > Besides if we really need to fix the order here, the fix
> > should be somewhere at the level of cgroup_file_release() or even
> > kernfs to work for other similar situations.
>
> Good point but cgroup and kernfs have no idea of psi trigger.

Yes, that's why I think if we really need to fix the order here and do
it properly, it won't be straightforward. IMHO wake_up_pollfree() is
an appropriate and simple fix for this.

>
> The bonus of the uaf is check polled file upon release in scenarios like
> the psi trigger.
>