Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221108110715.227062-1-pedro.falcato@gmail.com> <202211101934.22CACD615@keescook>
In-Reply-To: <202211101934.22CACD615@keescook>
From:   Pedro Falcato <pedro.falcato@gmail.com>
Date:   Fri, 11 Nov 2022 03:59:08 +0000
Message-ID: <CAKbZUD2r=zK91J4c6pyHQhDuqqMzmrx+BBgFF4Cso2jK+jjhHA@mail.gmail.com>
Subject: Re: [PATCH v3] fs/binfmt_elf: Fix memsz > filesz handling
To:     Kees Cook <keescook@chromium.org>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org, dalias@libc.org,
        ebiederm@xmission.com, sam@gentoo.org, viro@zeniv.linux.org.uk
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Nov 11, 2022 at 3:38 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Nov 08, 2022 at 11:07:15AM +0000, Pedro Falcato wrote:
> > [...]
> > +              * This tail logic is skippable if we're the last phdr, as
> > +              * nothing can map an address >= our p_vaddr, since ELF phdr
> > +              * PT_LOAD segments are required to be sorted in an increasing
> > +              * order.
>
> I'm still looking through the patch, but I do want to call this bit out
> as a problem. The kernel cannot, unfortunately, make this assumption. See:
> https://lore.kernel.org/linux-fsdevel/YfOooXQ2ScpZLhmD@fractal.localdomain/

Ugh. I guess it doesn't matter in this situation? That logic only
matters if we're trying to fix this new loading bug, and old
executables load correctly with the old behavior anyway,
which is what you get if that logic falls through.

I don't know if this makes sense, but in my (possibly naive) opinion
we have a compromise to keep loading what could already be loaded,
without actually requiring to load broken ELFs 100% correctly.

We could of course also just sort the program headers at load time,
but I assume that's unwanted overhead for most well behaved ELF
program headers :)

-- 
Pedro Falcato