linux-kernel - Re: [PATCH 00 of 10] x86: unify asm/pgtable.h

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: <20080109014233.GG25945@bingen.suse.de>
Date:	Wed, 9 Jan 2008 02:42:33 +0100
From:	Andi Kleen <ak@...e.de>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Andi Kleen <ak@...e.de>, Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>,
	Glauber de Oliveira Costa <glommer@...il.com>,
	Jan Beulich <jbeulich@...ell.com>
Subject: Re: [PATCH 00 of 10] x86: unify asm/pgtable.h

On Tue, Jan 08, 2008 at 05:35:26PM -0800, Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
>>> of the if()?  If not, can't we just remove it and avoid this present 
>>> problem?
>>>     
>>
>> That would just hide your problem.
>>   
>
> The "problem" is a BUG() in pageattr_64.c:change_page_attr(), which to me 
> looks spurious.  It arises because __PAGE_KERNEL_* doesn't contain 
> _PAGE_GLOBAL, but PAGE_KERNEL_* does.  When ioremap() change_page_attr(), 
> it does so in a way that guarentees that the test
>
> 	if (pgprot_val(prot) != pgprot_val(ref_prot)) {
>
> in __change_page_attr() always succeeds.  When I folded _PAGE_GLOBAL into 
> the __PAGE_KERNEL_* definitions, it mostly works except it causes this if() 
> to fail, falling into the otherwise dead else clause and triggers a BUG().

That means your callers pass in the wrong values.

> I'm not really sure what the logic in here is supposed to be doing, but it 
> seems to me that it isn't the difference between global and non-global 
> mappings (I guess its really testing for cached vs uncached mappings).

No it is testing for all page attributes. That can be cached vs uncached
but also rw vs ro and nx vs non nx and everything else you can set
as bits on ptes.

While changing global sounds weird the pageattr-test code I recently
submitted does just that for test purposes.


> Or to put it another way, what's the underlying rationale for making 
> __PAGE_KERNEL_* not include the GLOBAL flag, but including it in the pgprot 
> versions?  It means that code like this in ioremap_64.c:

There is none, but that is not what change_page_attr() cares about.
It just cares that you pass in the correct bits and you don't currently.

I think you're missing the forrest because of all the trees currently.

-Andi

--
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/