linux-kernel - Re: [tip:x86/mm] x86/mm/mtrr: Clean up mtrr_type

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Jul 2015 15:18:02 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	mingo@...nel.org, hpa@...or.com, bp@...en8.de, dvlasenk@...hat.com,
	bp@...e.de, akpm@...ux-foundation.org, brgerst@...il.com,
	tglx@...utronix.de, linux-mm@...ck.org, luto@...capital.net,
	mcgrof@...e.com, toshi.kani@...com, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Cc:	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/mm] x86/mm/mtrr: Clean up mtrr_type_lookup()

On Wed, May 27, 2015 at 07:19:05AM -0700, tip-bot for Toshi Kani wrote:
> +/**
> + * mtrr_type_lookup - look up memory type in MTRR
> + *
> + * Return Values:
> + * MTRR_TYPE_(type)  - The effective MTRR type for the region
> + * MTRR_TYPE_INVALID - MTRR is disabled
>   */
>  u8 mtrr_type_lookup(u64 start, u64 end)
>  {

>  	int repeat;
>  	u64 partial_end;
>  
> +	if (!mtrr_state_set)
> +		return MTRR_TYPE_INVALID;
> +
> +	if (!(mtrr_state.enabled & MTRR_STATE_MTRR_ENABLED))
> +		return MTRR_TYPE_INVALID;
> +
> +	/*
> +	 * Look up the fixed ranges first, which take priority over
> +	 * the variable ranges.
> +	 */
> +	if ((start < 0x100000) &&
> +	    (mtrr_state.have_fixed) &&
> +	    (mtrr_state.enabled & MTRR_STATE_MTRR_FIXED_ENABLED))
> +		return mtrr_type_lookup_fixed(start, end);
> +
> +	/*
> +	 * Look up the variable ranges.  Look of multiple ranges matching
> +	 * this address and pick type as per MTRR precedence.
> +	 */
> +	type = mtrr_type_lookup_variable(start, end, &partial_end, &repeat);
>  
>  	/*
>  	 * Common path is with repeat = 0.
>  	 * However, we can have cases where [start:end] spans across some
> +	 * MTRR ranges and/or the default type.  Do repeated lookups for
> +	 * that case here.
>  	 */
>  	while (repeat) {
>  		prev_type = type;
>  		start = partial_end;
> +		type = mtrr_type_lookup_variable(start, end, &partial_end, &repeat);
>  
>  		if (check_type_overlap(&prev_type, &type))
>  			return type;
>  	}
>  
> +	if (mtrr_tom2 && (start >= (1ULL<<32)) && (end < mtrr_tom2))
> +		return MTRR_TYPE_WRBACK;
> +
>  	return type;
>  }

So I got staring at this MTRR horror show because I _really_ _Really_
want to kill stop_machine_from_inactive_cpu().

But I wondered about these lookup functions, should they not have an
assertion that preemption is disabled?

Using these functions with preemption enabled is racy against MTRR
updates. And if that race is ok, at the very least explain that it is
indeed racy and why this is not a problem.
--
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/