linux-kernel - Re: [PATCH] x86/tools: fix line number reported for malformed lines

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]

Date: Wed, 21 Feb 2024 14:19:33 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Valentin Obst <kernel@...entinobst.de>
Cc: Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>, 
	Andreas Hindborg <a.hindborg@...sung.com>, John Baublitz <john.m.baublitz@...il.com>, 
	David Rheinsberg <david@...dahead.eu>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"H. Peter Anvin" <hpa@...or.com>, Masami Hiramatsu <mhiramat@...nel.org>, Miguel Ojeda <ojeda@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, 
	Sergio González Collado <sergio.collado@...il.com>, 
	Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org, x86@...nel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH] x86/tools: fix line number reported for malformed lines

On Wed, Feb 21, 2024 at 10:00 AM Valentin Obst <kernel@...entinobstde> wrote:
>
> While debugging the `X86_DECODER_SELFTEST` failure first reported in [1],
> [In this case the line causing the failure is interpreted as two lines by
> the tool (due to its length, but this is fixed by [1, 2]), and the second
> line is reported. Still the spatial closeness between the reported line and
> the line causing the failure would have made debugging a lot easier.]

Thanks Valentin, John et al. for digging into this (and the related
issue) -- very much appreciated.

It looks good to me:

Reviewed-by: Miguel Ojeda <ojeda@...nel.org>
Tested-by: Miguel Ojeda <ojeda@...nel.org>

This should probably have a Fixes tag -- from a quick look, the
original test did not seem to have the problem because `insns` was
equivalent to the number of lines since there was no `if ... {
continue; }` for the symbol case. At some point that branch was added,
so that was not true anymore, thus that one should probably be the
Fixes tag, though please double-check:

    Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed")

It is a minor issue for sure, so perhaps not worth backporting, but
nevertheless the hash is in a very old kernel, and thus the issue
applies to all stable kernels. So it does not hurt flagging it to the
stable team:

    Cc: stable@...r.kernel.org

In addition, John reported the original issue, but this one was found
due to that one, and I am not exactly sure who did what here. Please
consider whether one of the following (or similar) may be fair:

    Reported-by: John Baublitz <john.m.baublitz@...il.com>
    Debugged-by: John Baublitz <john.m.baublitz@...il.com>

An extra Link to the discussion in Zulip could be nice too:

    Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/insn_decoder_test.20failure/near/421075039

Finally, a nit: links are typically written like the following -- you
can still use bracket references at the end:

    Link: https://lore.kernel.org/lkml/Y9ES4UKl%2F+DtvAVS@gmail.com/T/ [1]
    Link: https://lore.kernel.org/rust-for-linux/20231119180145.157455-1-sergio.collado@gmail.com/
[2]

Cheers,
Miguel