| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CA+CK2bADcVsRnovkwWftPCbubXoaFrPzSavMU+G9f3XAz3YMLQ@mail.gmail.com>
Date: Wed, 19 Nov 2025 16:56:10 -0500
From: Pasha Tatashin <pasha.tatashin@...een.com>
To: Mike Rapoport <rppt@...nel.org>
Cc: pratyush@...nel.org, jasonmiu@...gle.com, graf@...zon.com,
dmatlack@...gle.com, rientjes@...gle.com, corbet@....net,
rdunlap@...radead.org, ilpo.jarvinen@...ux.intel.com, kanie@...ux.alibaba.com,
ojeda@...nel.org, aliceryhl@...gle.com, masahiroy@...nel.org,
akpm@...ux-foundation.org, tj@...nel.org, yoann.congal@...le.fr,
mmaurer@...gle.com, roman.gushchin@...ux.dev, chenridong@...wei.com,
axboe@...nel.dk, mark.rutland@....com, jannh@...gle.com,
vincent.guittot@...aro.org, hannes@...xchg.org, dan.j.williams@...el.com,
david@...hat.com, joel.granados@...nel.org, rostedt@...dmis.org,
anna.schumaker@...cle.com, song@...nel.org, linux@...ssschuh.net,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-mm@...ck.org,
gregkh@...uxfoundation.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
rafael@...nel.org, dakr@...nel.org, bartosz.golaszewski@...aro.org,
cw00.choi@...sung.com, myungjoo.ham@...sung.com, yesanishhere@...il.com,
Jonathan.Cameron@...wei.com, quic_zijuhu@...cinc.com,
aleksander.lobakin@...el.com, ira.weiny@...el.com,
andriy.shevchenko@...ux.intel.com, leon@...nel.org, lukas@...ner.de,
bhelgaas@...gle.com, wagi@...nel.org, djeffery@...hat.com,
stuart.w.hayes@...il.com, ptyadav@...zon.de, lennart@...ttering.net,
brauner@...nel.org, linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
saeedm@...dia.com, ajayachandra@...dia.com, jgg@...dia.com, parav@...dia.com,
leonro@...dia.com, witu@...dia.com, hughd@...gle.com, skhawaja@...gle.com,
chrisl@...nel.org
Subject: Re: [PATCH v6 15/20] mm: memfd_luo: allow preserving memfd
On Mon, Nov 17, 2025 at 6:04 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> On Sat, Nov 15, 2025 at 06:34:01PM -0500, Pasha Tatashin wrote:
> > From: Pratyush Yadav <ptyadav@...zon.de>
> >
> > The ability to preserve a memfd allows userspace to use KHO and LUO to
> > transfer its memory contents to the next kernel. This is useful in many
> > ways. For one, it can be used with IOMMUFD as the backing store for
> > IOMMU page tables. Preserving IOMMUFD is essential for performing a
> > hypervisor live update with passthrough devices. memfd support provides
> > the first building block for making that possible.
> >
> > For another, applications with a large amount of memory that takes time
> > to reconstruct, reboots to consume kernel upgrades can be very
> > expensive. memfd with LUO gives those applications reboot-persistent
> > memory that they can use to quickly save and reconstruct that state.
> >
> > While memfd is backed by either hugetlbfs or shmem, currently only
> > support on shmem is added. To be more precise, support for anonymous
> > shmem files is added.
> >
> > The handover to the next kernel is not transparent. All the properties
> > of the file are not preserved; only its memory contents, position, and
> > size. The recreated file gets the UID and GID of the task doing the
> > restore, and the task's cgroup gets charged with the memory.
> >
> > Once preserved, the file cannot grow or shrink, and all its pages are
> > pinned to avoid migrations and swapping. The file can still be read from
> > or written to.
> >
> > Use vmalloc to get the buffer to hold the folios, and preserve
> > it using kho_preserve_vmalloc(). This doesn't have the size limit.
> >
> > Co-developed-by: Pasha Tatashin <pasha.tatashin@...een.com>
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
> > Signed-off-by: Pratyush Yadav <ptyadav@...zon.de>
>
> The order of signed-offs seems wrong, Pasha's should be the last one.
Updated.
> > + * This interface is a contract. Any modification to the FDT structure,
> > + * node properties, compatible string, or the layout of the serialization
> > + * structures defined here constitutes a breaking change. Such changes require
> > + * incrementing the version number in the MEMFD_LUO_FH_COMPATIBLE string.
>
> The same comment about contract as for the generic LUO documentation
> applies here (https://lore.kernel.org/all/aRnG8wDSSAtkEI_z@kernel.org/)
Added.
>
> > + *
> > + * FDT Structure Overview:
> > + * The memfd state is contained within a single FDT with the following layout:
>
> ...
>
> > +static struct memfd_luo_folio_ser *memfd_luo_preserve_folios(struct file *file, void *fdt,
> > + u64 *nr_foliosp)
> > +{
>
> If we are already returning nr_folios by reference, we might do it for
> memfd_luo_folio_ser as well and make the function return int.
Done
>
> > + struct inode *inode = file_inode(file);
> > + struct memfd_luo_folio_ser *pfolios;
> > + struct kho_vmalloc *kho_vmalloc;
> > + unsigned int max_folios;
> > + long i, size, nr_pinned;
> > + struct folio **folios;
>
> pfolios and folios read like the former is a pointer to latter.
> I'd s/pfolios/folios_ser/
Done
> > + int err = -EINVAL;
> > + pgoff_t offset;
> > + u64 nr_folios;
>
> ...
>
> > + kvfree(folios);
> > + *nr_foliosp = nr_folios;
> > + return pfolios;
> > +
> > +err_unpreserve:
> > + i--;
> > + for (; i >= 0; i--)
>
> Maybe a single line
>
> for (--i; i >= 0; --i)
Done, but wrote it as:
for (i = i - 1; i >= 0; i--)
Which looks a little cleaner to me.
>
> > + kho_unpreserve_folio(folios[i]);
> > + vfree(pfolios);
> > +err_unpin:
> > + unpin_folios(folios, nr_folios);
> > +err_free_folios:
> > + kvfree(folios);
> > + return ERR_PTR(err);
> > +}
> > +
> > +static void memfd_luo_unpreserve_folios(void *fdt, struct memfd_luo_folio_ser *pfolios,
> > + u64 nr_folios)
> > +{
> > + struct kho_vmalloc *kho_vmalloc;
> > + long i;
> > +
> > + if (!nr_folios)
> > + return;
> > +
> > + kho_vmalloc = (struct kho_vmalloc *)fdt_getprop(fdt, 0, MEMFD_FDT_FOLIOS, NULL);
> > + /* The FDT was created by this kernel so expect it to be sane. */
> > + WARN_ON_ONCE(!kho_vmalloc);
>
> The FDT won't have FOLIOS property if size was zero, will it?
> I think that if we add kho_vmalloc handle to struct memfd_luo_private and
> pass that around it will make things easier and simpler.
I am actually thinking of removing FDTs and using versioned struct directly.
>
> > + kho_unpreserve_vmalloc(kho_vmalloc);
> > +
> > + for (i = 0; i < nr_folios; i++) {
> > + const struct memfd_luo_folio_ser *pfolio = &pfolios[i];
> > + struct folio *folio;
> > +
> > + if (!pfolio->foliodesc)
> > + continue;
>
> How can this happen? Can pfolios be a sparse array?
With the current implementation of memfd_pin_folios, which populates
holes, this array will be dense. This check is defensive coding in
case we switch to a sparse preservation mechanism in the future. I
will add a comment, and add a warn_on_once.
>
> > + folio = pfn_folio(PRESERVED_FOLIO_PFN(pfolio->foliodesc));
> > +
> > + kho_unpreserve_folio(folio);
> > + unpin_folio(folio);
> > + }
> > +
> > + vfree(pfolios);
> > +}
>
> ...
>
> > +static void memfd_luo_finish(struct liveupdate_file_op_args *args)
> > +{
> > + const struct memfd_luo_folio_ser *pfolios;
> > + struct folio *fdt_folio;
> > + const void *fdt;
> > + u64 nr_folios;
> > +
> > + if (args->retrieved)
> > + return;
> > +
> > + fdt_folio = memfd_luo_get_fdt(args->serialized_data);
> > + if (!fdt_folio) {
> > + pr_err("failed to restore memfd FDT\n");
> > + return;
> > + }
> > +
> > + fdt = folio_address(fdt_folio);
> > +
> > + pfolios = memfd_luo_fdt_folios(fdt, &nr_folios);
> > + if (!pfolios)
> > + goto out;
> > +
> > + memfd_luo_discard_folios(pfolios, nr_folios);
>
> Does not this free the actual folios that were supposed to be preserved?
It does, when memfd was not reclaimed.
>
> > + vfree(pfolios);
> > +
> > +out:
> > + folio_put(fdt_folio);
> > +}
>
> ...
>
> > +static int memfd_luo_retrieve(struct liveupdate_file_op_args *args)
> > +{
> > + struct folio *fdt_folio;
> > + const u64 *pos, *size;
> > + struct file *file;
> > + int len, ret = 0;
> > + const void *fdt;
> > +
> > + fdt_folio = memfd_luo_get_fdt(args->serialized_data);
>
> Why do we need to kho_restore_folio() twice? Here and in
> memfd_luo_finish()?
Here we retrieve memfd and give it to userspace. In finish, discard
whatever was not reclaimed.
>
> > + if (!fdt_folio)
> > + return -ENOENT;
> > +
> > + fdt = page_to_virt(folio_page(fdt_folio, 0));
>
> folio_address()
Done
>
>
> --
> Sincerely yours,
> Mike.
Powered by blists - more mailing lists