Re: [PATCH v2 29/37] powerpc/nohash: Replace pte_user() by pte_read()

[Aneesh Kumar K.V]

Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:

> Le 07/11/2023 à 14:34, Aneesh Kumar K.V a écrit :
>> Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:
>> 
>>> Le 31/10/2023 à 11:15, Aneesh Kumar K.V a écrit :
>>>> Christophe Leroy <christophe.leroy@xxxxxxxxxx> writes:

....

>>
>> 
>> We are adding the pte flags check not the map addr check there. Something like this?
>
> Well, ok, but then why do we want to do that check for ioremap() and not 
> for everything else ? vmap() for instance will not perform any such 
> check. All it does is to clear the EXEC bit.
>
> As far as I can see, no other architecture does such a check, so why is 
> it needed on powerpc at all ?
>
> Regardless, comments below.
>

Looking at ioremap_prot() I am not clear whether we can really use the
flag value argument as is. For ex: x86 does 

pgprot2cachemode(__pgprot(prot_val))

I see that we use ioremap_prot() for generic_access_phys() and with
/dev/mem and __access_remote_vm() we can get called with a user pte
mapping prot flags? 

If such an prot value can be observed then the original change to clear
EXEC and mark it privileged is required?

	/* we don't want to let _PAGE_USER and _PAGE_EXEC leak out */
	pte = pte_exprotect(pte);
	pte = pte_mkprivileged(pte);


We already handle exec in pgprot_nx() and we need add back
pte_mkprivileged()? 


-aneesh