Re: [PATCH v6 3/6] fs/procfs: add build ID fetching to PROCMAP_QUERY API

[Andrii Nakryiko]

On Tue, Jul 2, 2024 at 7:50 AM Andi Kleen <ak@xxxxxxxxxxxxxxx> wrote:
>
> > 1) non-executable file-backed VMA still has build ID associated with
> > it. Note, build ID is extracted from the backing file's content, not
> > from VMA itself. The part of ELF file that contains build ID isn't
> > necessarily mmap()'ed at all
>
> That's true, but there should be at least one executable mapping
> for any useful ELF file.
>
> Basically such a check guarantee that you cannot tell anything
> about a non x mapping not related to ELF.

Hey Andi,

So when we were discussing this I was imagining that
inode/address_space does have something like VMA's VM_MAYEXEC flag and
it would be easy and fast to check that. But it doesn't seem so.

So what exactly did you have in mind when you were proposing that
check? Did you mean to do a pass over all VMAs within the process to
check if there is at least one executable VMA belonging to
address_space? If yes, then that would certainly be way too expensive
to be usable.

If I missed something obvious, please point me in the right direction.

As it stands, I don't see any reasonable way to check what you asked
performantly. And given this is a bit of over-cautious check, I'm
inclined to just not add it. Worst case someone with PTRACE_MODE_READ
access would be able to tell if the first 4 bytes of a file are ELF
signature or not. Given PTRACE_MODE_READ, I'd imagine that's not
really a problem.

>
> >
> > 2) What sort of exploitation are we talking about here? it's not
> > enough for backing file to have correct 4 starting bytes (0x7f"ELF"),
> > we still have to find correct PT_NOTE segment, and .note.gnu.build-id
> > section within it, that has correct type (3) and key name "GNU".
>
> There's a timing side channel, you can tell where the checks
> stop. I don't think it's a big problem, but it's still better to avoid
> such leaks in the first place as much as possible.
>
> >
> > I'm trying to understand what we are protecting against here.
> > Especially that opening /proc/<pid>/maps already requires
> > PTRACE_MODE_READ permissions anyways (or pid should be self).
>
> While that's true for the standard security permission model there might
> be non standard ones where the relationship is more complicated.
>
> -Andi