Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAJuCfpEMEsSYcKakFiDK=QV+apW-2baLcUcw7uRyrmKkWVnR8A@mail.gmail.com>
 <20230113022555.2467724-1-kamatam@amazon.com> <CAJuCfpEH7kC=S8S_SRLW-X483kpaL4xdn5b35Ou08V7b56QdJA@mail.gmail.com>
 <CAJuCfpHV2-pnHd6U3paA1fO2gaYP1RTqAJwp_5QC7C2YR3JG5g@mail.gmail.com>
 <CAJuCfpFZ3B4530TgsSHqp5F_gwfrDujwRYewKReJru==MdEHQg@mail.gmail.com>
 <20230120013055.3628-1-hdanton@sina.com> <CAJuCfpF9tCx4mjYFyX+p7qO9qt+rm=UMSdBt-uzaOqE0ThG04g@mail.gmail.com>
 <20230120090001.3807-1-hdanton@sina.com> <CAJuCfpFShuXs_CcfXR6PjYyXemapbuHPG0oZQJb_Y9ZpCUGZXg@mail.gmail.com>
 <20230121051746.4100-1-hdanton@sina.com>
In-Reply-To: <20230121051746.4100-1-hdanton@sina.com>
From:   Suren Baghdasaryan <surenb@google.com>
Date:   Sat, 21 Jan 2023 19:01:13 -0800
Message-ID: <CAJuCfpESYRYavQOkDYyJhurpEcV5ioUMb=i4Q0EJ1UpVpyRfAA@mail.gmail.com>
Subject: Re: another use-after-free in ep_remove_wait_queue()
To:     Hillf Danton <hdanton@sina.com>
Cc:     Munehisa Kamata <kamatam@amazon.com>, Tejun Heo <tj@kernel.org>,
        ebiggers@kernel.org, hannes@cmpxchg.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org, mengcc@amazon.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 20, 2023 at 9:18 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Fri, 20 Jan 2023 08:28:25 -0800 Suren Baghdasaryan <surenb@google.com>
> > On Fri, Jan 20, 2023 at 1:00 AM Hillf Danton <hdanton@sina.com> 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.
>