linux-kernel - Re: [PATCH] x86: skip check for spurious faults for non-present faults

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: <53752885.5080306@intel.com>
Date:	Thu, 15 May 2014 13:50:13 -0700
From:	Dave Hansen <dave.hansen@...el.com>
To:	David Vrabel <david.vrabel@...rix.com>,
	linux-kernel@...r.kernel.org
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org
Subject: Re: [PATCH] x86: skip check for spurious faults for non-present faults

On 05/12/2014 03:29 AM, David Vrabel wrote:
> -	/* Reserved-bit violation or user access to kernel space? */
> -	if (error_code & (PF_USER | PF_RSVD))
> +	/* Only check for spurious faults on supervisor write or
> +	   instruction faults. */
> +	if (error_code != (PF_WRITE | PF_PROT)
> +	    && error_code != (PF_INSTR | PF_PROT))
>  		return 0;

This changes the semantics a bit too much for me to feel happy about it.
 This is at best missing quite a bit of detail from the changelog.

 1. 'return 0' means "this was not a spurious fault"
 2. We used to check for the presence of PF_USER|PF_RSVD
 3. This patch checks now for two _explicit_ conditions, which
    implicitly check for the _absence_ of the two bits we checked for
    before.

I do believe your patch is correct, but it took me a bit to convince
myself that it was the right thing.  Please be explicit (in the comment)
about the exact PTE transitions that you expect to get you here.

Also, I have to wonder if you can just leave the original if() in there.
 You're making this _more_ restrictive than it was before, and I wonder
if it might just be more clear if you have both checks.  The compiler
might even compile it down to the same code, just changing the immediate
that was generated for the mask that you're checking.
--
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/