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;
Date:   Tue, 20 Jun 2023 09:50:00 -0700
From:   Omar Sandoval <osandov@osandov.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Josh Poimboeuf <jpoimboe@kernel.org>, linux-kernel@vger.kernel.org,
        linux-debuggers@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v2] x86/unwind/orc: add ELF section with ORC version
 identifier
Message-ID: <ZJHYuH/DD6tqJ+bu@telecaster>
References: <aef9c8dc43915b886a8c48509a12ec1b006ca1ca.1686690801.git.osandov@osandov.com>
 <20230614091751.GE1639749@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230614091751.GE1639749@hirez.programming.kicks-ass.net>
Precedence: bulk

On Wed, Jun 14, 2023 at 11:17:51AM +0200, Peter Zijlstra wrote:
> On Tue, Jun 13, 2023 at 02:14:56PM -0700, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.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@fb.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?

Ping, Josh, any chance of getting this in to 6.4? Sorry to be cutting it
so close.