[PATCH] binfmt_elf_fdpic: only honour the first PT_INTERP

[PATCH] binfmt_elf_fdpic: only honour the first PT_INTERP

[Christian Brauner]

The program header scan handles PT_INTERP from a switch nested in the
scan loop, so its break leaves the switch and not the loop. A binary
carrying more than one PT_INTERP runs the case again and overwrites both
interpreter_name and interpreter. The previous name allocation leaks and
so does the previous interpreter reference, along with the write denial
open_exec() took on it. The denial is never released, so the file stays
unwritable for as long as the system runs.

An unprivileged caller reaches this with a crafted binary and repeats it
at will. binfmt_elf stops at the first PT_INTERP. Do the same here.

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").

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/binfmt_elf_fdpic.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 7e3108489c83..fe0b5c5ed2bc 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -231,6 +231,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 	for (i = 0; i < exec_params.hdr.e_phnum; i++, phdr++) {
 		switch (phdr->p_type) {
 		case PT_INTERP:
+			/* elf ABI allows only one interpreter */
+			if (interpreter_name)
+				continue;
+
 			retval = -ENOMEM;
 			if (phdr->p_filesz > PATH_MAX)
 				goto error;
-- 
2.53.0

Re: [PATCH] binfmt_elf_fdpic: only honour the first PT_INTERP

[Jori Koolstra]

On Tue, Jul 21, 2026 at 01:20:45PM +0200, Christian Brauner wrote:
> The program header scan handles PT_INTERP from a switch nested in the
> scan loop, so its break leaves the switch and not the loop. A binary
> carrying more than one PT_INTERP runs the case again and overwrites both
> interpreter_name and interpreter. The previous name allocation leaks and
> so does the previous interpreter reference, along with the write denial
> open_exec() took on it. The denial is never released, so the file stays
> unwritable for as long as the system runs.
> 
> An unprivileged caller reaches this with a crafted binary and repeats it
> at will. binfmt_elf stops at the first PT_INTERP. Do the same here.
> 
> 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").
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> ---
>  fs/binfmt_elf_fdpic.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index 7e3108489c83..fe0b5c5ed2bc 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -231,6 +231,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
>  	for (i = 0; i < exec_params.hdr.e_phnum; i++, phdr++) {
>  		switch (phdr->p_type) {
>  		case PT_INTERP:
> +			/* elf ABI allows only one interpreter */
> +			if (interpreter_name)
> +				continue;
> +
>  			retval = -ENOMEM;
>  			if (phdr->p_filesz > PATH_MAX)
>  				goto error;
> -- 
> 2.53.0
> 

Good catch.

Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>

Btw, it seems that elf_fdpic_map_file_constdisp_on_uclinux() has no
check to verify that phdr->p_memsz >= phdr->p_filesz, despite elf(5)
saying that

	The file size may not be larger than the memory size.

Afaict, currently read_code() may blow past what vm_mmap() allocated
there, which may result in memory corruption for a non MMU system. This
is an LLM assisted find, and it may be that I missed something when
verifying. If you agree this is wrong, I'll fix it.

Thanks,
Jori.

Re: [PATCH] binfmt_elf_fdpic: only honour the first PT_INTERP

[Christian Brauner]

On 2026-07-21 22:37 +0200, Jori Koolstra wrote:
> On Tue, Jul 21, 2026 at 01:20:45PM +0200, Christian Brauner wrote:
> > The program header scan handles PT_INTERP from a switch nested in the
> > scan loop, so its break leaves the switch and not the loop. A binary
> > carrying more than one PT_INTERP runs the case again and overwrites both
> > interpreter_name and interpreter. The previous name allocation leaks and
> > so does the previous interpreter reference, along with the write denial
> > open_exec() took on it. The denial is never released, so the file stays
> > unwritable for as long as the system runs.
> > 
> > An unprivileged caller reaches this with a crafted binary and repeats it
> > at will. binfmt_elf stops at the first PT_INTERP. Do the same here.
> > 
> > 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").
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> > ---
> >  fs/binfmt_elf_fdpic.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> > index 7e3108489c83..fe0b5c5ed2bc 100644
> > --- a/fs/binfmt_elf_fdpic.c
> > +++ b/fs/binfmt_elf_fdpic.c
> > @@ -231,6 +231,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
> >  	for (i = 0; i < exec_params.hdr.e_phnum; i++, phdr++) {
> >  		switch (phdr->p_type) {
> >  		case PT_INTERP:
> > +			/* elf ABI allows only one interpreter */
> > +			if (interpreter_name)
> > +				continue;
> > +
> >  			retval = -ENOMEM;
> >  			if (phdr->p_filesz > PATH_MAX)
> >  				goto error;
> > -- 
> > 2.53.0
> > 
> 
> Good catch.
> 
> Reviewed-by: Jori Koolstra <jkoolstra@xs4all.nl>
> 
> Btw, it seems that elf_fdpic_map_file_constdisp_on_uclinux() has no
> check to verify that phdr->p_memsz >= phdr->p_filesz, despite elf(5)
> saying that
> 
> 	The file size may not be larger than the memory size.
> 
> Afaict, currently read_code() may blow past what vm_mmap() allocated
> there, which may result in memory corruption for a non MMU system. This
> is an LLM assisted find, and it may be that I missed something when
> verifying. If you agree this is wrong, I'll fix it.

Seems like a decent fix to do.