MIME-Version: 1.0
References: <20220106232513.143014-1-akirakawata1@gmail.com>
In-Reply-To: <20220106232513.143014-1-akirakawata1@gmail.com>
From:   Lukas Bulwahn <lukas.bulwahn@gmail.com>
Date:   Fri, 7 Jan 2022 07:40:50 +0100
Message-ID: <CAKXUXMwzULZHmfx5T74cjG++gd8mFKVOR7Z4aS8RabKnXWGOdQ@mail.gmail.com>
Subject: Re: [PATCH v4 RESEND 0/2] fs/binfmt_elf: Fix AT_PHDR for unusual ELF files
To:     Akira Kawata <akirakawata1@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Kees Cook <keescook@chromium.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Eric Biederman <ebiederm@xmission.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 7, 2022 at 12:25 AM Akira Kawata <akirakawata1@gmail.com> wrote:
>
>  These patches fix a bug in AT_PHDR calculation.
>
>  We cannot calculate AT_PHDR as the sum of load_addr and exec->e_phoff.
>  This is because exec->e_phoff is the offset of PHDRs in the file and the
>  address of PHDRs in the memory may differ from it. These patches fix the
>  bug by calculating the address of program headers from PT_LOADs
>  directly.
>
>  Sorry for my latency.
>
>  Changes in v4
>  - Reflecting comments from Lukas, add a refactoring commit.
>

Thanks for removing the dead store with your refactoring as a small
stylistic change, but I really think that Kees Cook's comment that you
simply removed an important feature is much more important to address.
There was no reply to that and it seems that Kees hypothesis that the
feature has been removed, was not questioned by anyone.

Lukas

>  Changes in v3:
>  - Fix a reported bug from kernel test robot.
>
>  Changes in v2:
>  - Remove unused load_addr from create_elf_tables.
>  - Improve the commit message. *** SUBJECT HERE ***
>
> Akira Kawata (2):
>   fs/binfmt_elf: Fix AT_PHDR for unusual ELF files
>   fs/binfmt_elf: Refactor load_elf_binary function
>
>  fs/binfmt_elf.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
>
>
> base-commit: 4eee8d0b64ecc3231040fa68ba750317ffca5c52
> --
> 2.25.1
>