[PATCH] binfmt_elf_fdpic: reject PT_LOAD with filesz larger than memsz

[PATCH] binfmt_elf_fdpic: reject PT_LOAD with filesz larger than memsz

[Jérémy Jean]

The ELF specification requires p_filesz to be no larger than p_memsz for
PT_LOAD segments.

elf_fdpic_map_file_constdisp_on_uclinux() sizes its contiguous allocation
from p_memsz, then read_code() copies p_filesz bytes into it. A malformed
segment can therefore copy file contents past the allocation on NOMMU
systems. The direct-mmap path also subtracts p_filesz from p_memsz without
first validating the relationship.

Validate every PT_LOAD immediately after fetching the program headers.
This covers both executable and interpreter headers before
begin_new_exec() makes execution irreversible.

On RV32 NOMMU, an ET_DYN with an 8192-byte p_filesz and 4096-byte p_memsz
copied a marker from the second file page past its one-page mapping.
After this change execve() rejects it with -EINVAL, while an 8192/8192
control still executes.

The flaw dates back to the driver's introduction in the pre-git history
tree introduced in v2.6.11 by 91808d6ebe39 ("[PATCH] FRV: Add FDPIC ELF
binary format driver").

Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 fs/binfmt_elf_fdpic.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 068c46875c74..e8a6e89b76a0 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -157,6 +157,12 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
 	if (unlikely(retval != size))
 		return retval < 0 ? retval : -ENOEXEC;
 
+	phdr = params->phdrs;
+	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
+		if (phdr->p_type == PT_LOAD && phdr->p_filesz > phdr->p_memsz)
+			return -EINVAL;
+	}
+
 	/* determine stack size for this binary */
 	phdr = params->phdrs;
 	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
-- 
2.47.3

Re: [PATCH] binfmt_elf_fdpic: reject PT_LOAD with filesz larger than memsz

[Jori Koolstra]

On Thu, Aug 20, 2026 at 10:28:26PM +0000, Jérémy Jean wrote:
> The ELF specification requires p_filesz to be no larger than p_memsz for
> PT_LOAD segments.
> 
> elf_fdpic_map_file_constdisp_on_uclinux() sizes its contiguous allocation
> from p_memsz, then read_code() copies p_filesz bytes into it. A malformed
> segment can therefore copy file contents past the allocation on NOMMU
> systems. The direct-mmap path also subtracts p_filesz from p_memsz without
> first validating the relationship.
> 
> Validate every PT_LOAD immediately after fetching the program headers.
> This covers both executable and interpreter headers before
> begin_new_exec() makes execution irreversible.
> 
> On RV32 NOMMU, an ET_DYN with an 8192-byte p_filesz and 4096-byte p_memsz
> copied a marker from the second file page past its one-page mapping.
> After this change execve() rejects it with -EINVAL, while an 8192/8192
> control still executes.
> 
> The flaw dates back to the driver's introduction in the pre-git history
> tree introduced in v2.6.11 by 91808d6ebe39 ("[PATCH] FRV: Add FDPIC ELF
> binary format driver").

I came across this about a month ago. But I wonder if this is even worth
fixing. You are on a non-MMU system, so running a malicious ELF kinda
means gameover anyhow, doesn't it?

> 
> Assisted-by: Codex:gpt-5

You are allowed to use LLM assistance, but the commit message also reads
like AI, so I am a bit worried whether you actually understand the
change you are proposing. Afaik, this is still required.

> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  fs/binfmt_elf_fdpic.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index 068c46875c74..e8a6e89b76a0 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -157,6 +157,12 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
>  	if (unlikely(retval != size))
>  		return retval < 0 ? retval : -ENOEXEC;
>  
> +	phdr = params->phdrs;
> +	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
> +		if (phdr->p_type == PT_LOAD && phdr->p_filesz > phdr->p_memsz)
> +			return -EINVAL;
> +	}
> +
>  	/* determine stack size for this binary */
>  	phdr = params->phdrs;
>  	for (loop = 0; loop < params->hdr.e_phnum; loop++, phdr++) {
> -- 
> 2.47.3
> 

Re: [PATCH] binfmt_elf_fdpic: reject PT_LOAD with filesz larger than memsz

[Jérémy Jean]

Hello Jori,

On 2026-08-21 13:45, Jori Koolstra wrote:
> On Thu, Aug 20, 2026 at 10:28:26PM +0000, Jérémy Jean wrote:
>> The ELF specification requires p_filesz to be no larger than p_memsz 
>> for
>> PT_LOAD segments.
>> 
>> elf_fdpic_map_file_constdisp_on_uclinux() sizes its contiguous 
>> allocation
>> from p_memsz, then read_code() copies p_filesz bytes into it. A 
>> malformed
>> segment can therefore copy file contents past the allocation on NOMMU
>> systems. The direct-mmap path also subtracts p_filesz from p_memsz 
>> without
>> first validating the relationship.
>> 
>> Validate every PT_LOAD immediately after fetching the program headers.
>> This covers both executable and interpreter headers before
>> begin_new_exec() makes execution irreversible.
>> 
>> On RV32 NOMMU, an ET_DYN with an 8192-byte p_filesz and 4096-byte 
>> p_memsz
>> copied a marker from the second file page past its one-page mapping.
>> After this change execve() rejects it with -EINVAL, while an 8192/8192
>> control still executes.
>> 
>> The flaw dates back to the driver's introduction in the pre-git 
>> history
>> tree introduced in v2.6.11 by 91808d6ebe39 ("[PATCH] FRV: Add FDPIC 
>> ELF
>> binary format driver").
> 
> I came across this about a month ago. But I wonder if this is even 
> worth
> fixing. You are on a non-MMU system, so running a malicious ELF kinda
> means gameover anyhow, doesn't it?

Well, this would act as defense in depth, no? I agree that cases where 
this
matters are probably rare, if any, but I don't see why this should not 
be fixed
anyway. Isn't it best not to assume anything about the use cases? The 
patch is
only a suggestion to start discussions: I'm merely raising this to your
attention but you guys (maintainers) are obviously in the best place to 
check
whether there are better options.

Regards,
Jérémy