From:   ebiederm@xmission.com (Eric W. Biederman)
To:     "H.J. Lu" <hjl.tools@gmail.com>
Cc:     linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexey Dobriyan <adobriyan@gmail.com>
References: <20211212204043.231568-1-hjl.tools@gmail.com>
Date:   Mon, 13 Dec 2021 10:40:47 -0600
In-Reply-To: <20211212204043.231568-1-hjl.tools@gmail.com> (H. J. Lu's message
        of "Sun, 12 Dec 2021 12:40:43 -0800")
Message-ID: <87wnk8xxtc.fsf@email.froward.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH v2] fs/binfmt_elf.c: disallow invalid entry point address
Precedence: bulk

"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Linux, the start of the first PT_LOAD segment is the ELF header and
> the address 0 points to the ELF magic bytes.  Update the ELF loader to
> disallow ELF binaries with entry point address smaller than the ELF
> header size.

I kind of get why that was suggested but there is most definitely no
requirement for the program headers to be loaded let alone be at any
particular virtual address.  We could be talking about a static elf
binary.

> This fixes:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215303
>
> Tested by booting Fedora 35 and running a shared library with invalid
> entry point address:
>
> $ readelf -h load.so | grep "Entry point address:"
>   Entry point address:               0x4
> $ ./load.so
> bash: ./load.so: cannot execute binary file: Exec format error
> $

Is the point of this keeping shared libraries from executing?
What is gained by this patch?

Eric


> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  fs/binfmt_elf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index bd78587194dc..7f035022131b 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 < sizeof(*elf_ex))
> +		goto out;
>  	if (!elf_check_arch(elf_ex))
>  		goto out;
>  	if (elf_check_fdpic(elf_ex))