According to gABI, the entry point address in the ELF header gives the
virtual address to which the system first transfers control, thus
starting the process. If the file has no associated entry point, this
member holds zero. Update the ELF loader to disallow an ELF binary
with zero entry point address. This fixes:
https://bugzilla.kernel.org/show_bug.cgi?id=215303
Tested by booting Fedora 35 and running a shared library with zero entry
point address:
$ readelf -h load.so | grep "Entry point address:"
Entry point address: 0x0
$ ./load.so
bash: ./load.so: cannot execute binary file: Exec format error
$
Signed-off-by: H.J. Lu <[email protected]>
---
fs/binfmt_elf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index bd78587194dc..bb427c97dc02 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -850,6 +850,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (elf_ex->e_type != ET_EXEC && elf_ex->e_type != ET_DYN)
goto out;
+ if (elf_ex->e_entry == 0)
+ goto out;
if (!elf_check_arch(elf_ex))
goto out;
if (elf_check_fdpic(elf_ex))
--
2.33.1
On 12/11/21, H.J. Lu <[email protected]> wrote:
> According to gABI, the entry point address in the ELF header gives the
> virtual address to which the system first transfers control, thus
> starting the process. If the file has no associated entry point, this
> member holds zero. Update the ELF loader to disallow an ELF binary
> with zero entry point address. This fixes:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215303
>
> Tested by booting Fedora 35 and running a shared library with zero entry
> point address:
>
> $ readelf -h load.so | grep "Entry point address:"
> Entry point address: 0x0
> $ ./load.so
> bash: ./load.so: cannot execute binary file: Exec format error
Why not let it segfault?
> + if (elf_ex->e_entry == 0)
> + goto out;
On Sat, Dec 11, 2021 at 11:38 PM Alexey Dobriyan <[email protected]> wrote:
>
> On 12/11/21, H.J. Lu <[email protected]> wrote:
> > According to gABI, the entry point address in the ELF header gives the
> > virtual address to which the system first transfers control, thus
> > starting the process. If the file has no associated entry point, this
> > member holds zero. Update the ELF loader to disallow an ELF binary
> > with zero entry point address. This fixes:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=215303
> >
> > Tested by booting Fedora 35 and running a shared library with zero entry
> > point address:
> >
> > $ readelf -h load.so | grep "Entry point address:"
> > Entry point address: 0x0
> > $ ./load.so
> > bash: ./load.so: cannot execute binary file: Exec format error
>
> Why not let it segfault?
>
> > + if (elf_ex->e_entry == 0)
> > + goto out;
Why let it segfault?
--
H.J.
On Sun, Dec 12, 2021 at 5:52 AM H.J. Lu <[email protected]> wrote:
>
> On Sat, Dec 11, 2021 at 11:38 PM Alexey Dobriyan <[email protected]> wrote:
> >
> > Why not let it segfault?
>
> Why let it segfault?
That's not my main worry - what if somebody has a code section with a
zero vaddr and intentionally put the entry at the beginning?
Maybe it's not supposed to work by some paper standatd, but afaik
currently it _would_ work.
All these things are relative to the load address, so a zero e_entry
doesn't mean NULL, and may be a perfectly valid address.
No?
Linus
On Sun, Dec 12, 2021 at 10:29 AM Linus Torvalds
<[email protected]> wrote:
>
> On Sun, Dec 12, 2021 at 5:52 AM H.J. Lu <[email protected]> wrote:
> >
> > On Sat, Dec 11, 2021 at 11:38 PM Alexey Dobriyan <[email protected]> wrote:
> > >
> > > Why not let it segfault?
> >
> > Why let it segfault?
>
> That's not my main worry - what if somebody has a code section with a
> zero vaddr and intentionally put the entry at the beginning?
>
> Maybe it's not supposed to work by some paper standatd, but afaik
> currently it _would_ work.
>
> All these things are relative to the load address, so a zero e_entry
> doesn't mean NULL, and may be a perfectly valid address.
>
> No?
According to the ELF specification, zero entry point value means
there is no entry point. Such ELF binary doesn't conform to the
ELF specification.
--
H.J.
On Sun, Dec 12, 2021 at 11:06 AM H.J. Lu <[email protected]> wrote:
>
> According to the ELF specification, zero entry point value means
> there is no entry point. Such ELF binary doesn't conform to the
> ELF specification.
Nobody cares about paper specifications.
All that matters is REALITY.
So let me quote my email again, since you clearly didn't actually read
it (read that "maybe it's not supposed to work" part):
> That's not my main worry - what if somebody has a code section with a
> zero vaddr and intentionally put the entry at the beginning?
>
> Maybe it's not supposed to work by some paper standatd, but afaik
> currently it _would_ work.
I'm not sure this can happen currently (maybe all tools effectively
make it so that the ELF headers etc are part of the loaded image).
But no, paper specifications have absolutely no meaning if they don't
match realty.
And the reality is that I don't think we've ever checked e_entry being
zero, which means that maybe people have used it.
So convince me that the above cannot happen. I'm perfectly willing to
be convinced, but "some random paper standard that we've never
followed" is not the thing to quote.
Linus
On Sun, Dec 12, 2021 at 11:15 AM Linus Torvalds
<[email protected]> wrote:
>
> On Sun, Dec 12, 2021 at 11:06 AM H.J. Lu <[email protected]> wrote:
> >
> > According to the ELF specification, zero entry point value means
> > there is no entry point. Such ELF binary doesn't conform to the
> > ELF specification.
>
> Nobody cares about paper specifications.
>
> All that matters is REALITY.
>
> So let me quote my email again, since you clearly didn't actually read
> it (read that "maybe it's not supposed to work" part):
>
> > That's not my main worry - what if somebody has a code section with a
> > zero vaddr and intentionally put the entry at the beginning?
> >
> > Maybe it's not supposed to work by some paper standatd, but afaik
> > currently it _would_ work.
>
> I'm not sure this can happen currently (maybe all tools effectively
> make it so that the ELF headers etc are part of the loaded image).
>
> But no, paper specifications have absolutely no meaning if they don't
> match realty.
>
> And the reality is that I don't think we've ever checked e_entry being
> zero, which means that maybe people have used it.
>
> So convince me that the above cannot happen. I'm perfectly willing to
> be convinced, but "some random paper standard that we've never
> followed" is not the thing to quote.
>
On Linux, the start of the first PT_LOAD segment is the ELF
header and the address 0 points to the ELF magic bytes which
isn't a valid code sequence.
--
H.J.
On Sun, Dec 12, 2021 at 11:15 AM Linus Torvalds
<[email protected]> wrote:
>
> I'm not sure this can happen currently (maybe all tools effectively
> make it so that the ELF headers etc are part of the loaded image).
Side note: if that ends up being the case (ie e_entry always
effectively relative to the head of the image), then I think a better
fix would be to make that explicit, something like
if (elf_ex->e_entry < header_sizes)
goto out;
but the logic on exactly how things get loaded is so messy that I'm
not sure just what the situation is.
We've had things like old tool chains generate messy binaries before,
to the point where we've had to revert much more important changes (ie
the whole mess with MAP_FIXED_NOREPLACE and overlapping sections).
Linus
[ Crossed emails ]
On Sun, Dec 12, 2021 at 11:30 AM H.J. Lu <[email protected]> wrote:
>
> On Linux, the start of the first PT_LOAD segment is the ELF
> header and the address 0 points to the ELF magic bytes which
> isn't a valid code sequence.
Yeah, then I think a much more valid argument (and patch) is _that_ argument.
So that kind of explanation, along with a patch more along the line of that
if (elf_ex->e_entry < header_sizes)
goto out;
I suggested, and not talking about paper standards that may or may not
be relevant.
That would be much more palatable to me - it's a _technical_ argument,
not a "some paper standard that we clearly have never followed"
argument.
Linus
On Sun, Dec 12, 2021 at 11:35:56AM -0800, Linus Torvalds wrote:
> [ Crossed emails ]
>
> On Sun, Dec 12, 2021 at 11:30 AM H.J. Lu <[email protected]> wrote:
> >
> > On Linux, the start of the first PT_LOAD segment is the ELF
> > header and the address 0 points to the ELF magic bytes which
> > isn't a valid code sequence.
>
> Yeah, then I think a much more valid argument (and patch) is _that_ argument.
>
> So that kind of explanation, along with a patch more along the line of that
>
> if (elf_ex->e_entry < header_sizes)
> goto out;
>
> I suggested, and not talking about paper standards that may or may not
> be relevant.
>
> That would be much more palatable to me - it's a _technical_ argument,
> not a "some paper standard that we clearly have never followed"
> argument.
>
> Linus
I sent out the v2 patch with
if (elf_ex->e_entry < sizeof(*elf_ex))
goto out;
H.J.
On Sun, Dec 12, 2021 at 05:52:14AM -0800, H.J. Lu wrote:
> On Sat, Dec 11, 2021 at 11:38 PM Alexey Dobriyan <[email protected]> wrote:
> >
> > On 12/11/21, H.J. Lu <[email protected]> wrote:
> > > According to gABI, the entry point address in the ELF header gives the
> > > virtual address to which the system first transfers control, thus
> > > starting the process. If the file has no associated entry point, this
> > > member holds zero. Update the ELF loader to disallow an ELF binary
> > > with zero entry point address. This fixes:
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=215303
> > >
> > > Tested by booting Fedora 35 and running a shared library with zero entry
> > > point address:
> > >
> > > $ readelf -h load.so | grep "Entry point address:"
> > > Entry point address: 0x0
> > > $ ./load.so
> > > bash: ./load.so: cannot execute binary file: Exec format error
> >
> > Why not let it segfault?
> >
> > > + if (elf_ex->e_entry == 0)
> > > + goto out;
>
> Why let it segfault?
Such babysitting adds a branch for everyone to catch small number of
binaries.
e_entry can point to kernelspace, and it should segfault on the first
instruction (correctly).
It can iincorrectly point to unmapped area or VMA with wrong permissions,
with the same effect. But now check is more costly.