linux-kernel - Re: [git pull] x86 fixes

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:	Thu, 27 Mar 2008 13:31:43 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Ingo Molnar <mingo@...e.hu>
cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [git pull] x86 fixes



On Thu, 27 Mar 2008, Ingo Molnar wrote:
>
> Ingo Molnar (1):
>       x86: fix prefetch workaround
...
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index fdc6674..c0c82bc 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -92,7 +92,8 @@ static int is_prefetch(struct pt_regs *regs, unsigned long addr,
>  	unsigned char *max_instr;
>  
>  #ifdef CONFIG_X86_32
> -	if (!(__supported_pte_mask & _PAGE_NX))
> +	/* Catch an obscure case of prefetch inside an NX page: */
> +	if ((__supported_pte_mask & _PAGE_NX) && (error_code & 16))
>  		return 0;
>  #endif

Ingo, this patch makes no sense.

Two reasons:

 - "error_code & 16" is senseless. Use PF_INSTR instead, which actually 
   tells the reader something.

 - this piece of crap code is immediately followed by

	/* If it was a exec fault on NX page, ignore */
	if (error_code & PF_INSTR)
		return 0;

   which uses that *right* and readable PF_INSTR #define, and also shows 
   that the newly modified code is totally insane (ie: if the new code 
   triggers, then it would have returned 0 later _anyway_)

So I think it's just crap. I think it's duplication from the merging of 
the x86 code, and I think that the fact that the new code didn't use the 
right #define helper means that people didn't see that it was crap.

I pulled it, but this needs some resolution. The code makes no sense. 
Should that #ifdef and the code inside of it just be removed entirely?

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/