linux-kernel - Re: [PATCH v3] 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]

Message-Id: <20240403091748.ee180a7a1d4bf92e0c46fb8a@kernel.org>
Date: Wed, 3 Apr 2024 09:17:48 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Valentin Obst <kernel@...entinobst.de>
Cc: Borislav Petkov <bp@...en8.de>, Dave Hansen
 <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>, Ingo
 Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>, Thomas Gleixner
 <tglx@...utronix.de>, Andreas Hindborg <a.hindborg@...sung.com>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, John Baublitz
 <john.m.baublitz@...il.com>, Masami Hiramatsu <mhiramat@...nel.org>, Miguel
 Ojeda <ojeda@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Sergio
 González Collado <sergio.collado@...il.com>,
 linux-kernel@...r.kernel.org, x86@...nel.org, stable@...r.kernel.org
Subject: Re: [PATCH v3] x86/tools: fix line number reported for malformed
 lines

On Fri, 29 Mar 2024 12:31:58 +0100
Valentin Obst <kernel@...entinobst.de> wrote:

> Commit 35039eb6b199 ("x86: Show symbol name if insn decoder test failed")
> included symbol lines in the post-processed objdump output consumed by
> the insn decoder test. This broke the `instuction lines == total lines`
> property that `insn_decoder_test.c` relied upon to print the offending
> line's number in error messages. This has the consequence that the line
> number reported on a test failure is unreated to, and much smaller than,
> the line that actually caused the problem.
> 
> Add a new variable that counts the combined (insn+symbol) line count and
> report this in the error message.

This looks good to me. Thanks!

Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>

> 
> Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed")
> Cc: stable@...r.kernel.org
> Reviewed-by: Miguel Ojeda <ojeda@...nel.org>
> Tested-by: Miguel Ojeda <ojeda@...nel.org>
> Reported-by: John Baublitz <john.m.baublitz@...il.com>
> Debugged-by: John Baublitz <john.m.baublitz@...il.com>
> Signed-off-by: Valentin Obst <kernel@...entinobst.de>
> ---
> See v2's commit message and [1] for context why this bug made debugging a
> test failure harder than necessary.
> 
> [1]: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/insn_decoder_test.20failure/near/421075039
> 
> Changes in v3:
> - Add Cc stable tag in sign-off area.
> - Make commit message less verbose.
> - Link to v2: https://lore.kernel.org/r/20240223-x86-insn-decoder-line-fix-v2-1-cde49c69f402@valentinobst.de
> 
> Changes in v2:
> - Added tags 'Reviewed-by', 'Tested-by', 'Reported-by', 'Debugged-by',
>   'Link', and 'Fixes'.
> - Explain why this patch fixes the commit mentioned in the 'Fixes' tag.
> - CCed the stable list and sent to all x86 maintainers.
> - Link to v1: https://lore.kernel.org/r/20240221-x86-insn-decoder-line-fix-v1-1-47cd5a1718c6@valentinobst.de
> ---
>  arch/x86/tools/insn_decoder_test.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c
> index 472540aeabc2..727017a3c3c7 100644
> --- a/arch/x86/tools/insn_decoder_test.c
> +++ b/arch/x86/tools/insn_decoder_test.c
> @@ -114,6 +114,7 @@ int main(int argc, char **argv)
>  	unsigned char insn_buff[16];
>  	struct insn insn;
>  	int insns = 0;
> +	int lines = 0;
>  	int warnings = 0;
> 
>  	parse_args(argc, argv);
> @@ -123,6 +124,8 @@ int main(int argc, char **argv)
>  		int nb = 0, ret;
>  		unsigned int b;
> 
> +		lines++;
> +
>  		if (line[0] == '<') {
>  			/* Symbol line */
>  			strcpy(sym, line);
> @@ -134,12 +137,12 @@ int main(int argc, char **argv)
>  		strcpy(copy, line);
>  		tab1 = strchr(copy, '\t');
>  		if (!tab1)
> -			malformed_line(line, insns);
> +			malformed_line(line, lines);
>  		s = tab1 + 1;
>  		s += strspn(s, " ");
>  		tab2 = strchr(s, '\t');
>  		if (!tab2)
> -			malformed_line(line, insns);
> +			malformed_line(line, lines);
>  		*tab2 = '\0';	/* Characters beyond tab2 aren't examined */
>  		while (s < tab2) {
>  			if (sscanf(s, "%x", &b) == 1) {
> 
> ---
> base-commit: 4cece764965020c22cff7665b18a012006359095
> change-id: 20240221-x86-insn-decoder-line-fix-7b1f2e1732ff
> 
> Best regards,
> --
> Valentin Obst <kernel@...entinobst.de>
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>