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

[Christophe Leroy]

Le 13/11/2023 à 11:23, Aneesh Kumar K.V a écrit :
> Christophe Leroy <christophe.leroy at csgroup.eu> writes:
> 
>> Le 07/11/2023 à 14:34, Aneesh Kumar K.V a écrit :
>>> Christophe Leroy <christophe.leroy at csgroup.eu> writes:
>>>
>>>> Le 31/10/2023 à 11:15, Aneesh Kumar K.V a écrit :
>>>>> Christophe Leroy <christophe.leroy at csgroup.eu> 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?

Do you think so ?

If I understand correctly, in those cases ioremap_prot() is called with 
prot flags as returned by the call to phys_mem_access_prot().

> 
> 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()?

If this is the case for powerpc that's likely the case for most 
architectures. Should we change pgprot_nx() to pgprot_nxu() or have a 
pgprot_nu() in addition ?

Christophe