[Devel] [PATCH hci-8.1 0/2] mm, binfmt_elf: two fixes also posted to mainline

[Devel] [PATCH hci-8.1 0/2] mm, binfmt_elf: two fixes also posted to mainline

[Konstantin Khorenko]

Two independent fixes for bugs that mainline has as well. Both are
posted to mainline; the backport notes name what differs in this tree.
The MADV_DOEXEC series that follow (2 to 5) apply on top of this one,
and series 4 relies on patch 1.

Mainline
========

- Patch 1 is patch 1/2 of "mm/huge_memory: fix the copy of special
  PMDs at fork", v3:
  https://lore.kernel.org/r/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v3-0-85eaec8417f6@virtuozzo.com
  Sashiko review:
  https://sashiko.dev/#/patchset/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v3-0-85eaec8417f6@virtuozzo.com
  The Sashiko review of v1 found the ppc64 issue that patch 2/2 there
  fixes:
  https://sashiko.dev/#/patchset/20261014-b4-prep-vstor-149847-copy_huge_pmd-recheck-special-pmd-v1-1-d225c60bc316@virtuozzo.com
- Patch 2 is "binfmt_elf: return the real error when program headers
  cannot be loaded", v1:
  https://lore.kernel.org/r/20261014-b4-prep-vstor-149847-binfmt_elf-enomem-load-elf-phdrs-v1-1-f39c74d55e42@virtuozzo.com
  Sashiko review:
  https://sashiko.dev/#/patchset/20261014-b4-prep-vstor-149847-binfmt_elf-enomem-load-elf-phdrs-v1-1-f39c74d55e42@virtuozzo.com

Patch 1: copy_huge_pmd() recheck
================================

fork copies a special (PFN) huge PMD from a value read without the page
table lock. vfio-pci zaps its BAR mappings with unmap_mapping_range(),
which takes no mmap lock, when device memory is disabled or the device
is reset. A fork that races with that zap leaves the child with a BAR
mapping the zap removed: the child keeps reaching MMIO that vfio-pci has
disabled instead of getting SIGBUS. Series 4 copies these PMDs on every
live-update exec.

In mainline this patch is now the first of two. The second deposits a
page table for the copied PMD on ppc64 in hash mode; it is not needed
here, as this tree has no huge pfnmap support on ppc64.

Patch 2: the real error from load_elf_phdrs()
=============================================

load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
turns that into a fixed error: -ENOEXEC for the executable, -ELIBBAD
for its interpreter. The real cause is lost:

  cause                                  really      execve() returns
  -------------------------------------  ----------  ----------------
  bad e_phentsize, e_phnum 0 or too big  bad file    ENOEXEC (right)
  file ends before the header table      bad file    ENOEXEC (right)
  kmalloc() of the table fails           ENOMEM      ENOEXEC (wrong)
  kernel_read() fails                    EIO/ENOMEM  ENOEXEC (wrong)

For the interpreter every row gives ELIBBAD.

ENOEXEC says the file is not an executable, which is a property of the
file, not a transient failure:

- glibc's execvp() and execlp() take ENOEXEC as "not a binary" and run
  the ELF file with /bin/sh, which reports a binary it cannot execute
  instead of the ENOMEM or EIO.
- search_binary_handler() tries the next binary format instead of
  returning the error.
- For the interpreter the caller gets ELIBBAD, "Accessing a corrupted
  shared library", about an ld.so that is fine.

When it happens here: this tree lacks the v6.17 change that lets the
table grow past ELF_MIN_ALIGN, so the allocation fails in practice only
under fault injection; the read fails with an I/O error, or with
-ENOMEM when the page cache cannot take the pages.

Where it came from:

- Until commit 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header
  loading to a function") load_elf_binary() returned -ENOMEM and the
  read error. That commit documented an ERR_PTR return but returned
  NULL; commit cfc46ca4fdca later fixed the comment, not the code.
- Commit a9d9ef133f44 ("binfmt_elf: load interpreter program headers
  earlier") gave the interpreter's headers the same NULL check, and with
  it -ELIBBAD.

Testing
=======

On a KVM guest with the whole stack (series 1 to 5), release and debug
kernels:
- a fork-vs-revoke reproducer on a vfio-pci BAR (QXL in no-IOMMU mode):
  no stale child mapping; on mainline the same reproducer with a delay
  in the window shows the bug and that the patch closes it;
- failslab sweep over one execve(): ENOMEM instead of ENOEXEC/ELIBBAD,
  execvp() no longer falls back to /bin/sh; a header table whose second
  page fails to read gives EIO; truncated tables and e_phoff = 2^63
  still give ENOEXEC/ELIBBAD;
- selftests/exec, the MADV_DOEXEC selftests, gup_test, a Container
  start/exec/stop.

https://virtuozzo.atlassian.net/browse/VSTOR-150144

---
Konstantin Khorenko (2):
      mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()
      binfmt_elf: return the real error when program headers cannot be loaded

 fs/binfmt_elf.c  | 51 +++++++++++++++++++++++++++++++++------------------
 mm/huge_memory.c | 23 ++++++++++++++++-------
 2 files changed, 49 insertions(+), 25 deletions(-)
---
base-commit: defa968cbe8c38f1ff0231042dee07f00d27b2c9
change-id: 20261014-b4-prep-khorenko-vstor-149847-1-upstream-fixes-4bb22a75a079

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/2] mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()

[Konstantin Khorenko]

copy_huge_pmd() reads a special PMD without the page table lock and
copies that value to the child. The mmap write lock held by fork does
not stop unmap_mapping_range(): it zaps under i_mmap_rwsem and the PMD
lock.

The zap visits the parent VMA before the child's, as dup_mmap() links
the child right after the parent in i_mmap. If it clears the parent PMD
after fork has read it, and passes the child before fork copies it, the
child keeps a mapping the driver has revoked:

- vfio-pci: the child reaches the BAR after the device memory was
  disabled or the device was reset, instead of getting SIGBUS;
- panthor: the child maps the pages of an evicted buffer object after
  they are freed.

Read the PMD again under the source PMD lock, which the zap takes to
clear it. Either the zap got there first, and the PMD is none, or the
copy is in place before the zap reaches the child. Faults are locked
out, so only a zap can change the PMD, and pmd_none() is the test.

Backport note: in mainline this is patch 1 of a two-patch series. Here
the lockless check has no huge zero PMD test and the CoW warning uses
is_cow_mapping(), as this tree does. Commit bc02afbd4d73 ("mm/fork:
accept huge pfnmap entries") and the vfio-pci huge fault handler are in
this tree; panthor maps no huge PMDs here. Patch 2 of the mainline
series, which deposits a page table for the copy on ppc64 in hash mode,
is not needed: this tree has no huge pfnmap support on ppc64.

Fixes: bc02afbd4d73 ("mm/fork: accept huge pfnmap entries")
https://virtuozzo.atlassian.net/browse/VSTOR-150144

Feature: fix ms/mm
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 mm/huge_memory.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 90868f69ee338..dc254430d6929 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1618,13 +1618,22 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 		src_ptl = pmd_lockptr(src_mm, src_pmd);
 		spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
 		/*
-		 * No need to recheck the pmd, it can't change with write
-		 * mmap lock held here.
-		 *
-		 * Meanwhile, making sure it's not a CoW VMA with writable
-		 * mapping, otherwise it means either the anon page wrongly
-		 * applied special bit, or we made the PRIVATE mapping be
-		 * able to wrongly write to the backend MMIO.
+		 * The mmap and VMA write locks of the source keep faults
+		 * out, but unmap_mapping_range() does not take the mmap
+		 * lock and may have cleared the pmd since the lockless
+		 * read. Let copy_pmd_range() skip it then.
+		 */
+		pmd = pmdp_get(src_pmd);
+		if (unlikely(pmd_none(pmd))) {
+			spin_unlock(src_ptl);
+			spin_unlock(dst_ptl);
+			return -EAGAIN;
+		}
+		/*
+		 * Make sure it's not a CoW VMA with writable mapping,
+		 * otherwise it means either the anon page wrongly applied
+		 * special bit, or we made the PRIVATE mapping be able to
+		 * wrongly write to the backend MMIO.
 		 */
 		VM_WARN_ON_ONCE(is_cow_mapping(src_vma->vm_flags) && pmd_write(pmd));
 		goto set_pmd;

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/2] binfmt_elf: return the real error when program headers cannot be loaded

[Konstantin Khorenko]

load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
turns that into -ENOEXEC for the executable and -ELIBBAD for its
interpreter. So a failed allocation or read of the program headers
makes glibc's execvp() run the ELF file with /bin/sh, and reports a
healthy ld.so as a corrupted shared library.

Return an ERR_PTR instead, as binfmt_elf_fdpic does:

  -ENOEXEC     bad e_phentsize, e_phnum or e_phoff, or the file ends
               before the header table
  -ENOMEM      the allocation fails
  read error   kernel_read() fails

Malformed files keep getting -ENOEXEC and -ELIBBAD. kernel_read()
returns a short count when a later page fails, so read on until the
error or the end of the file shows. An e_phoff past LLONG_MAX - size
is rejected up front, as rw_verify_area() would turn it into -EINVAL.

Backport note: the same patch is prepared for mainline. The v6.17 change
"binfmt_elf: remove the 4k limitation of program header size" is not in
this tree, so the header table is still limited to ELF_MIN_ALIGN and the
allocation in practice fails only under fault injection; the read errors
and the e_phoff check apply as they are.

Fixes: 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to a function")
Fixes: a9d9ef133f44 ("binfmt_elf: load interpreter program headers earlier")
https://virtuozzo.atlassian.net/browse/VSTOR-150144

Feature: fix ms/exec
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 9190e5204152a..a5689a2844d43 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -512,39 +512,48 @@ static unsigned long maximum_alignment(struct elf_phdr *cmds, int nr)
  *
  * Loads ELF program headers from the binary file elf_file, which has the ELF
  * header pointed to by elf_ex, into a newly allocated array. The caller is
- * responsible for freeing the allocated data. Returns NULL upon failure.
+ * responsible for freeing the allocated data. Returns an ERR_PTR upon failure:
+ * -ENOEXEC for a malformed header or when the read reaches the end of the
+ * file, the allocation or read error otherwise.
  */
 static struct elf_phdr *load_elf_phdrs(const struct elfhdr *elf_ex,
 				       struct file *elf_file)
 {
-	struct elf_phdr *elf_phdata = NULL;
-	int retval = -1;
-	unsigned int size;
+	struct elf_phdr *elf_phdata;
+	unsigned int size, done;
+	loff_t pos;
+	ssize_t rv;
 
 	/*
 	 * If the size of this structure has changed, then punt, since
 	 * we will be doing the wrong thing.
 	 */
 	if (elf_ex->e_phentsize != sizeof(struct elf_phdr))
-		goto out;
+		return ERR_PTR(-ENOEXEC);
 
 	/* Sanity check the number of program headers... */
-	/* ...and their total size. */
+	/* ...their total size and their offset. */
 	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
-	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
-		goto out;
+	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN ||
+	    elf_ex->e_phoff > LLONG_MAX - size)
+		return ERR_PTR(-ENOEXEC);
 
 	elf_phdata = kmalloc(size, GFP_KERNEL);
 	if (!elf_phdata)
-		goto out;
-
-	/* Read in the program headers */
-	retval = elf_read(elf_file, elf_phdata, size, elf_ex->e_phoff);
+		return ERR_PTR(-ENOMEM);
 
-out:
-	if (retval) {
-		kfree(elf_phdata);
-		elf_phdata = NULL;
+	/*
+	 * Read in the program headers. A read that fails part way returns
+	 * what it got, so read on to see the error or the end of the file.
+	 */
+	pos = elf_ex->e_phoff;
+	for (done = 0; done < size; done += rv) {
+		rv = kernel_read(elf_file, (char *)elf_phdata + done,
+				 size - done, &pos);
+		if (rv <= 0) {
+			kfree(elf_phdata);
+			return ERR_PTR(rv ? rv : -ENOEXEC);
+		}
 	}
 	return elf_phdata;
 }
@@ -939,8 +948,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		goto out;
 
 	elf_phdata = load_elf_phdrs(elf_ex, bprm->file);
-	if (!elf_phdata)
+	if (IS_ERR(elf_phdata)) {
+		retval = PTR_ERR(elf_phdata);
 		goto out;
+	}
 
 	elf_ppnt = elf_phdata;
 	for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) {
@@ -1044,8 +1055,12 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		/* Load the interpreter program headers */
 		interp_elf_phdata = load_elf_phdrs(interp_elf_ex,
 						   interpreter);
-		if (!interp_elf_phdata)
+		if (IS_ERR(interp_elf_phdata)) {
+			if (PTR_ERR(interp_elf_phdata) != -ENOEXEC)
+				retval = PTR_ERR(interp_elf_phdata);
+			interp_elf_phdata = NULL;
 			goto out_free_dentry;
+		}
 
 		/* Pass PT_LOPROC..PT_HIPROC headers to arch code */
 		elf_property_phdata = NULL;

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/2] mm/huge_memory: recheck a special PMD under the lock in copy_huge_pmd()

[no-reply]

> copy_huge_pmd() reads a special PMD without the page table lock and
> copies that value to the child. The mmap write lock held by fork does
> not stop unmap_mapping_range(): it zaps under i_mmap_rwsem and the PMD
> lock.
> 
> The zap visits the parent VMA before the child's, as dup_mmap() links
> the child right after the parent in i_mmap. If it clears the parent PMD
> after fork has read it, and passes the child before fork copies it, the
> child keeps a mapping the driver has revoked:
> 
> - vfio-pci: the child reaches the BAR after the device memory was
>   disabled or the device was reset, instead of getting SIGBUS;
> - panthor: the child maps the pages of an evicted buffer object after
>   they are freed.
> 
> [ ... ]
> https://virtuozzo.atlassian.net/browse/VSTOR-150144
> 
> Feature: fix ms/mm
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-1-upstream-fixes-v1-0-7fde9ffbebe9@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/2] binfmt_elf: return the real error when program headers cannot be loaded

[no-reply]

> load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
> turns that into -ENOEXEC for the executable and -ELIBBAD for its
> interpreter. So a failed allocation or read of the program headers
> makes glibc's execvp() run the ELF file with /bin/sh, and reports a
> healthy ld.so as a corrupted shared library.
> 
> Return an ERR_PTR instead, as binfmt_elf_fdpic does:
> 
>   -ENOEXEC     bad e_phentsize, e_phnum or e_phoff, or the file ends
>                before the header table
>   -ENOMEM      the allocation fails
>   read error   kernel_read() fails
> 
> Malformed files keep getting -ENOEXEC and -ELIBBAD. kernel_read()
> returns a short count when a later page fails, so read on until the
> [ ... ]
> https://virtuozzo.atlassian.net/browse/VSTOR-150144
> 
> Feature: fix ms/exec
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261014-b4-prep-khorenko-vstor-149847-1-upstream-fixes-v1-0-7fde9ffbebe9@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel