Re: [PATCH] arm64: io: Reject present-invalid user prot in ioremap_prot()

[Zeng Heng <zengheng4@huawei.com>]

Hi Catalin,

On 2026/9/10 1:07, Catalin Marinas wrote:
> On Wed, Sep 09, 2026 at 02:04:50PM +0100, Will Deacon wrote:
>> On Wed, Sep 09, 2026 at 01:50:09PM +0100, Catalin Marinas wrote:
>>> On Sat, Sep 05, 2026 at 11:31:48AM +0800, Zeng Heng wrote:
>>>> From: Zeng Heng <zengheng4@huawei.com>
>>>>
>>>> Mapping a stack-top page via /dev/mem with PROT_NONE and then reading
>>>> that process's /proc/<pid>/cmdline triggers a spurious WARN in
>>>> ioremap_prot() through generic_access_phys():
>>>>
>>>>    WARNING: ./arch/arm64/include/asm/io.h:275 at generic_access_phys
>>>>    Call trace:
>>>>      generic_access_phys+0x1c8/0x228 (P)
>>>>      __access_remote_vm+0x2b4/0x398
>>>>      access_remote_vm+0x14/0x30
>>>>      get_mm_cmdline+0xf8/0x2a0
>>>>      proc_pid_cmdline_read+0x68/0x120
>>>>
>>>> generic_access_phys() passes the full pgprot derived from the user PTE
>>>> to ioremap_prot(). A PROT_NONE /dev/mem mapping is encoded as PAGE_NONE,
>>>> which clears PTE_VALID and sets the software PTE_PRESENT_INVALID bit.
>>>> On arm64 such an entry is still pte_present(), so follow_pfnmap_start()
>>>> reports the pfn and generic_access_phys() reaches ioremap_prot().
>>>> The PTE_USER assertion, which is meant to catch kernel prots being
>>>> passed by mistake, then fires for a PROT_NONE user mapping
>>>> that legitimately lacks PTE_USER, producing the spurious WARN.
>>>>
>>>> Reject a user prot encoding a present-invalid (i.e. PROT_NONE) entry up
>>>> front so that generic_access_phys() cleanly fails the access instead
>>>> of warning. Note that PTE_PRESENT_INVALID aliases the PTE_NG bit and
>>>> is only meaningful when PTE_VALID is clear, so both bits must be
>>>> checked together.
>>>>
>>>> Fixes: 8f098037139b ("arm64: io: Extract user memory type in ioremap_prot()")
>>>> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
>>>> ---
>>>>   arch/arm64/include/asm/io.h | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>>>> index 21c8e400107c..bbfbc4682639 100644
>>>> --- a/arch/arm64/include/asm/io.h
>>>> +++ b/arch/arm64/include/asm/io.h
>>>> @@ -272,6 +272,10 @@ static inline void __iomem *ioremap_prot(phys_addr_t phys, size_t size,
>>>>   	pgprot_t prot;
>>>>   	ptval_t user_prot_val = pgprot_val(user_prot);
>>>>   
>>>> +	if ((user_prot_val & (PTE_VALID | PTE_PRESENT_INVALID)) ==
>>>> +	     PTE_PRESENT_INVALID)
>>>> +		return NULL;
>>>> +
>>>>   	if (WARN_ON_ONCE(!(user_prot_val & PTE_USER)))
>>>>   		return NULL;
>>> I wonder whether we should just drop the warning and return NULL if
>>> !PTE_USER && PTE_UXN. The latter check would also catch execute-only
>>> mappings (Sashiko pointed out this case still trips the warning). The
>>> PROT_NONE case would be covered automatically as well since PTE_USER is
>>> cleared, PTE_UXN set. Maybe add a comment that that pte_protnone()
>>> relies on !PTE_USER && PTE_UXN, so it's not that we avoid the PROT_NONE
>>> issue by chance.
>> Hmm, do we want exec-only mappings to be readable via /dev/mem?
> Ah, yes, got confused on how it reaches this path. It doesn't make sense
> to allow exec-only to be readable. Also if we mmap(PROT_EXEC) /dev/mem,
> PTE_UXN ends up set anyway via pgprot_noncached(). But it does have
> PTE_VALID, so the above won't catch it.
>
> Checking !PTE_USER should be sufficient here and return NULL. For the
> warning, I think we can check PTE_NG first but it only works without
> kpti.

Thanks for your reply. I agree that checking PTE_USER is sufficient here.

I'll drop the WARN_ON_ONCE() and return NULL directly when PTE_USER
is not set. And I won't add a PTE_NG check for the warning since, as you
pointed out, that would not work reliably with KPTI.


Best regards,
Zeng Heng