Re: [PATCH 0/2] mm/memfd: add F_SEAL_PUNCH_HOLE

[Samiullah Khawaja]

On Sat, Oct 31, 2026 at 10:32:32AM +0100, David Hildenbrand (Arm) wrote:
> On 10/31/26 04:03, Samiullah Khawaja wrote:
> > On Fri, Oct 30, 2026 at 03:43:53PM -0700, Andy Lutomirski wrote:
> > > On Fri, Oct 30, 2026 at 10:40 AM Samiullah Khawaja <skhawaja@xxxxxxxxxx> wrote:
> > > > >On 10/30/26 13:11, Jason Gunthorpe wrote:
> > > > >> On Fri, Oct 30, 2026 at 11:20:30AM +0100, David Hildenbrand (Arm) wrote:
> > > > >>>
> > > > >>> We have the exact same situation when userspace maps anonymous folios
> > > > into vfio
> > > > >>> and then desides to MADV_DONTNEED the pages. Or any other memory backend
> > > > that is
> > > > >>> used for such purposes.
> > > > >>>
> > > > >>> User space must not zap/punch things that might be pinned elsewhere.
> > > > >>>
> > > > >>> That's not specific to memfd, udmabuf, iommufd or anything else.
> > > > >>>
> > > > >>> I don't see a reason to make userspace (that should not shoot itself into
> > > > the
> > > > >>> foot) a kernel feature it must activate itself to not shoot itself into
> > > > the foot.
> > > >
> > > > Agreed, for normal use this is userspace's problem. The seal was meant
> > > > for the kernel to cheaply verify at preserve time that nothing was
> > > > punched.
> > > >
> > > > >>
> > > > >> The issue is live update doesn't preserve those background punched
> > > > >> folios and now we have created an UAF attack on the kernel across
> > > > >> kexec if you don't do something about it.
> > > > >>
> > > > >> There was some notion here that with the right sealing flags we could
> > > > >> tell if everything was 'live update compatible' and then know that
> > > > >> these attacks are impossible.
> > > > >Just remember that there was any hole punch and refuse to live update this
> > > > instance.
> > > > >
> > > > >We could likely implement "we saw hole punch on a pinned folio" as well, but I
> > > > >don't think that's required initially.
> > > >
> > > > If I understand correctly, you mean recording on the memfd that a hole
> > > > punch happened, and refusing the live update if it did? That would work
> > > > and is much cheaper than what we do today, where iommufd walks all the
> > > > mapped pages at preserve time. For truncate, the existing F_SEAL_SHRINK
> > > > is enough or I can record that also on the memfd. I will send patches
> > > > for this.
> > >
> > > This entire situation continues to give me the willies.  After much
> > > prodding and contemplating (in my own brain), I got a friendly LLM to
> > > report:
> > >
> > > --- begin AI output ---
> > >
> > > IOMMU_IOAS_MAP_FILE uses memfd_pin_folios(). Collapse [MADV_COLLAPSE
> > > or khugepaged] checks the old folio’s reference count, so a pin
> > > acquired before that check makes collapse fail.
> > >
> > >  The problematic interleaving is:
> > >
> > >  1. collapse_file() locks and unmaps the old folio, then passes its
> > > reference-count check (mm/khugepaged.c:2446).
> > >  2. memfd_pin_folios() (mm/gup.c:3483) obtains the old folio from the
> > > file’s XArray and pins it, without taking the folio lock.
> > >  3. Collapse copies the contents and replaces the old cache entries
> > > with the new huge folio. It does not repeat the reference check or
> > > freeze the old count.
> > >  4. IOAS retains the old physical pages; accesses through the file
> > > use the new folio.
> > >
> > > --- end AI output ---
> > >
> > > Is this correct?  I haven't personally verified it.  But this whole
> >
> > Yes, I think this is correct. memfd_pin_folios() pins the folios
> > directly without taking the folio lock. Other page cache lookups lock
> > the folio and recheck folio->mapping, e.g. shmem_get_folio_gfp(). Only
> > memfd_pin_folios() misses it.
> >
> > This scenario is different from punch hole or MADV_DONTNEED where the
> > user decides to remove the pinned memory. Here the kernel is removing
> > the pinned page without the user knowing. Collapse already checks the
> > refcount to avoid this.
> >
> > The recent fix [1] by Chris already takes the folio lock for !uptodate
> > folios. I think we can take the lock unconditionally and recheck
> > folio->mapping, that should fix this. I can send a patch for this fix,
> > if folks agree.
>
> I have another patch from Anthropic on my "rework schlop" queue that should fix
> this:
>
> "mm/gup: pin memfd folios under the folio lock".
>
>
> We'd do just what we do in other context now, which is checking under folio lock
> whether the folio was truncated.
>
> Care to send a proper fix?

Sure, I will send a fix.

Thanks,
Sami