linux-kernel - Re: [PATCH v2] x86/unwind/orc: add ELF section with ORC version identifier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230614091751.GE1639749@hirez.programming.kicks-ass.net>
Date:   Wed, 14 Jun 2023 11:17:51 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Omar Sandoval <osandov@...ndov.com>
Cc:     Josh Poimboeuf <jpoimboe@...nel.org>, linux-kernel@...r.kernel.org,
        linux-debuggers@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v2] x86/unwind/orc: add ELF section with ORC version
 identifier

On Tue, Jun 13, 2023 at 02:14:56PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@...com>
> 
> Commits ffb1b4a41016 ("x86/unwind/orc: Add 'signal' field to ORC
> metadata") and fb799447ae29 ("x86,objtool: Split UNWIND_HINT_EMPTY in
> two") changed the ORC format. Although ORC is internal to the kernel,
> it's the only way for external tools to get reliable kernel stack traces
> on x86-64. In particular, the drgn debugger [1] uses ORC for stack
> unwinding, and these format changes broke it [2]. As the drgn
> maintainer, I don't care how often or how much the kernel changes the
> ORC format as long as I have a way to detect the change.
> 
> It suffices to store a version identifier in the vmlinux and kernel
> module ELF files (to use when parsing ORC sections from ELF), and in
> kernel memory (to use when parsing ORC from a core dump+symbol table).
> Rather than hard-coding a version number that needs to be manually
> bumped, Peterz suggested hashing the definitions from orc_types.h. If
> there is a format change that isn't caught by this, the hashing script
> can be updated.
> 
> This patch adds an .orc_header allocated ELF section containing the
> 20-byte hash to vmlinux and kernel modules, along with the corresponding
> __start_orc_header and __stop_orc_header symbols in vmlinux.
> 
> 1: https://github.com/osandov/drgn
> 2: https://github.com/osandov/drgn/issues/303
> 
> Signed-off-by: Omar Sandoval <osandov@...com>

Patch looks good to me; as a follow up I suppose we could verify the orc
hash on module load, to ensure the module and main kernel agree on the
ORC version used -- but we can do that later.

> ---
> Hi,
> 
> This is v2 of my patch to make it possible for external tools like drgn
> to identify versions of the ORC format. As stated in v1 [1], I don't
> want ORC to be stable ABI; I just need a way to identify the format
> being used.
> 
> This version incorporates Peter's suggestion to hash the ORC definitions
> instead of requiring a manual version number; this is easier to maintain
> and more resilient to backports.
> 
> I would love to get this in before 6.4 is released, and then hopefully
> backport it to 6.3-stable.

So we're fairly late in the cycle and it would need justification to go
into objtool/urgent -- preferably only fixes at this point.

But given we 'broke' the ORC layout this cycle, we can mark this with
Fixes: for the two mentioned commits.

Josh?