On Wed, Aug 13, 2025 at 09:41:40AM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 13, 2025 at 02:14:23PM +0200, Greg KH wrote:
> > On Wed, Aug 13, 2025 at 02:02:07PM +0200, Pratyush Yadav wrote:
> > > On Wed, Aug 13 2025, Greg KH wrote:
> > >
> > > > On Tue, Aug 12, 2025 at 11:34:37PM -0700, Vipin Sharma wrote:
> > > >> On 2025-08-07 01:44:35, Pasha Tatashin wrote:
> > > >> > From: Pratyush Yadav <ptyadav@xxxxxxxxx>
> > > >> > +static void memfd_luo_unpreserve_folios(const struct memfd_luo_preserved_folio *pfolios,
> > > >> > + unsigned int nr_folios)
> > > >> > +{
> > > >> > + unsigned int i;
> > > >> > +
> > > >> > + for (i = 0; i < nr_folios; i++) {
> > > >> > + const struct memfd_luo_preserved_folio *pfolio = &pfolios[i];
> > > >> > + struct folio *folio;
> > > >> > +
> > > >> > + if (!pfolio->foliodesc)
> > > >> > + continue;
> > > >> > +
> > > >> > + folio = pfn_folio(PRESERVED_FOLIO_PFN(pfolio->foliodesc));
> > > >> > +
> > > >> > + kho_unpreserve_folio(folio);
> > > >>
> > > >> This one is missing WARN_ON_ONCE() similar to the one in
> > > >> memfd_luo_preserve_folios().
> > > >
> > > > So you really want to cause a machine to reboot and get a CVE issued for
> > > > this, if it could be triggered? That's bold :)
> > > >
> > > > Please don't. If that can happen, handle the issue and move on, don't
> > > > crash boxes.
> > >
> > > Why would a WARN() crash the machine? That is what BUG() does, not
> > > WARN().
> >
> > See 'panic_on_warn' which is enabled in a few billion Linux systems
> > these days :(
>
> This has been discussed so many times already:
>
> https://lwn.net/Articles/969923/
>
> When someone tried to formalize this "don't use WARN_ON" position
> in the coding-style.rst it was NAK'd:
>
> https://lwn.net/ml/linux-kernel/10af93f8-83f2-48ce-9bc3-80fe4c60082c@xxxxxxxxxx/
>
> Based on Linus's opposition to the idea:
>
> https://lore.kernel.org/all/CAHk-=wgF7K2gSSpy=m_=K3Nov4zaceUX9puQf1TjkTJLA2XC_g@xxxxxxxxxxxxxx/
>
> Use the warn ons. Make sure they can't be triggered by userspace. Use
> them to detect corruption/malfunction in the kernel.
>
> In this case if kho_unpreserve_folio() fails in this call chain it
> means some error unwind is wrongly happening out of sequence, and we
> are now forced to leak memory. Unwind is not something that userspace
> should be controlling, so of course we want a WARN_ON here.
"should be" is the key here. And it's not obvious from this patch if
that's true or not, which is why I mentioned it.
I will keep bringing this up, given the HUGE number of CVEs I keep
assigning each week for when userspace hits WARN_ON() calls until that
flow starts to die out either because we don't keep adding new calls, OR
we finally fix them all. Both would be good...
thanks,
greg k-h
