* Re: [Question] About the elf program header size
2025-06-27 16:35 ` Kees Cook
@ 2025-06-30 0:12 ` YinFengwei
2025-07-02 1:08 ` YinFengwei
1 sibling, 0 replies; 4+ messages in thread
From: YinFengwei @ 2025-06-30 0:12 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-mm, linux-kernel, zhourundong.zrd
On Fri, Jun 27, 2025 at 09:35:45AM +0800, Kees Cook wrote:
> On Fri, Jun 27, 2025 at 09:04:11AM +0800, YinFengwei wrote:
> > We had a script generated assembly code. built it with gcc and the
> > output elf file had 78 program headers.
>
> Why so many?
I don't know the detail. Just know it's a tool generates assembly
code according to the trace data. From the objdump, it looks like
there are many hole generated (I assume it just generates the code
which is the code path just hit).
>
> > On an arm64 platform, if we have 64KB base page size, the elf can
> > be started correctly. But if we have 4KB base page size, the elf
> > can NOT be started with:
> > cannot execute binary file: Exec format error
> >
> > Look at the function load_elf_phdrs():
> > if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> > goto out;
> >
> > ELF_MIN_ALIGN is defined as PAGE_SIZE on arm64. Which can explain
> > above inconsistent behaviors (from user perspetive).
> >
> > I didn't find the limitation definition in ELF spec(Maybe I missed
> > some obvious info there). If I remove "size > ELF_MIN_ALIGN", the
> > same elf can be started correctly even with 4KB page size.
> >
> > So my question is why we limit the who program headers total size
> > to PAGE_SIZE? git history couldn't tell anything because the
> > limitation was introduced when whole linux kernel tree was migrated
> > to git. Is there a possible constrain on other architecture? Thanks.
>
> Looking through
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git
> (which doesn't have linked history, so you have to examine explicit "pre
> git" tags), I see:
>
> 4779b38bcb96 ("[PATCH] Linux-0.99.13 (September 19, 1993)")
> Which says "ELF binary support it a notable change." Here, the PAGE_SIZE
> check does not exist. When ELF interp support was added in
> 9e11983a5a3e ("Import 0.99.15f"), we see the check appear, and I can
> find no rationale.
>
> And with 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to
> a function"), the PAGE_SIZE check is _added_ for non-interp loads.
Thanks a lot for this information. I didn't know the pre-git history can
be found here.
>
> It seems the 64K count limit is sufficient? (If the goal was to avoid
> large memory allocations happening from userspace, we're way past
> PAGE_SIZE these days between IPC, BPF, etc.) Does this work for you?
Yes. It works good. Thanks.
Regards
Yin, Fengwei
>
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index a43363d593e5..92de44b8765f 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -519,7 +519,7 @@ static struct elf_phdr *load_elf_phdrs(const struct elfhdr *elf_ex,
> /* Sanity check the number of program headers... */
> /* ...and their total size. */
> size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
> - if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> + if (size == 0 || size > 65536)
> goto out;
>
> elf_phdata = kmalloc(size, GFP_KERNEL);
>
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Question] About the elf program header size
2025-06-27 16:35 ` Kees Cook
2025-06-30 0:12 ` YinFengwei
@ 2025-07-02 1:08 ` YinFengwei
1 sibling, 0 replies; 4+ messages in thread
From: YinFengwei @ 2025-07-02 1:08 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-mm, linux-kernel, zhourundong.zrd
On Fri, Jun 27, 2025 at 09:35:45AM +0800, Kees Cook wrote:
> On Fri, Jun 27, 2025 at 09:04:11AM +0800, YinFengwei wrote:
> > We had a script generated assembly code. built it with gcc and the
> > output elf file had 78 program headers.
>
> Why so many?
>
> > On an arm64 platform, if we have 64KB base page size, the elf can
> > be started correctly. But if we have 4KB base page size, the elf
> > can NOT be started with:
> > cannot execute binary file: Exec format error
> >
> > Look at the function load_elf_phdrs():
> > if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> > goto out;
> >
> > ELF_MIN_ALIGN is defined as PAGE_SIZE on arm64. Which can explain
> > above inconsistent behaviors (from user perspetive).
> >
> > I didn't find the limitation definition in ELF spec(Maybe I missed
> > some obvious info there). If I remove "size > ELF_MIN_ALIGN", the
> > same elf can be started correctly even with 4KB page size.
> >
> > So my question is why we limit the who program headers total size
> > to PAGE_SIZE? git history couldn't tell anything because the
> > limitation was introduced when whole linux kernel tree was migrated
> > to git. Is there a possible constrain on other architecture? Thanks.
>
> Looking through
> https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git
> (which doesn't have linked history, so you have to examine explicit "pre
> git" tags), I see:
>
> 4779b38bcb96 ("[PATCH] Linux-0.99.13 (September 19, 1993)")
> Which says "ELF binary support it a notable change." Here, the PAGE_SIZE
> check does not exist. When ELF interp support was added in
> 9e11983a5a3e ("Import 0.99.15f"), we see the check appear, and I can
> find no rationale.
>
> And with 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to
> a function"), the PAGE_SIZE check is _added_ for non-interp loads.
>
> It seems the 64K count limit is sufficient? (If the goal was to avoid
> large memory allocations happening from userspace, we're way past
> PAGE_SIZE these days between IPC, BPF, etc.) Does this work for you?
>
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index a43363d593e5..92de44b8765f 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -519,7 +519,7 @@ static struct elf_phdr *load_elf_phdrs(const struct elfhdr *elf_ex,
> /* Sanity check the number of program headers... */
> /* ...and their total size. */
> size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
> - if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN)
> + if (size == 0 || size > 65536)
> goto out;
>
> elf_phdata = kmalloc(size, GFP_KERNEL);
Just want to check: are you going to push this change to be merged?
Thanks.
Regards
Yin, Fengwei
>
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread