linux-kernel - Re: [PATCH] riscv: mm: Remove the copy operation of pmd

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] [day] [month] [year] [list]

Message-ID: <d10ea4ba-0aa5-4c70-8c3a-4a970d61f4fb@ghiti.fr>
Date:   Wed, 24 Feb 2021 07:34:55 -0500
From:   Alex Ghiti <alex@...ti.fr>
To:     Chuanhua Han <hanchuanhua@...hsemi.com>, paul.walmsley@...ive.com,
        palmer@...belt.com, aou@...s.berkeley.edu
Cc:     sfr@...b.auug.org.au, schwab@...e.de, Anup.Patel@....com,
        linux-kernel@...r.kernel.org, peterx@...hat.com,
        linux-riscv@...ts.infradead.org, ebiederm@...ssion.com,
        akpm@...ux-foundation.org, shihpo.hung@...ive.com,
        tglx@...utronix.de
Subject: Re: [PATCH] riscv: mm: Remove the copy operation of pmd

Le 3/30/20 à 7:53 AM, Chuanhua Han a écrit :
> Since all processes share the kernel address space,
> we only need to copy pgd in case of a vmalloc page
> fault exception, the other levels of page tables are
> shared, so the operation of copying pmd is unnecessary.
> 
> Signed-off-by: Chuanhua Han <hanchuanhua@...hsemi.com>
> ---
>   arch/riscv/mm/fault.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index be84e32adc4c..24f4ebfd2df8 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -208,9 +208,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
>   vmalloc_fault:
>   	{
>   		pgd_t *pgd, *pgd_k;
> -		pud_t *pud, *pud_k;
> -		p4d_t *p4d, *p4d_k;
> -		pmd_t *pmd, *pmd_k;
> +		pud_t *pud_k;
> +		p4d_t *p4d_k;
> +		pmd_t *pmd_k;
>   		pte_t *pte_k;
>   		int index;
>   
> @@ -234,12 +234,10 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
>   			goto no_context;
>   		set_pgd(pgd, *pgd_k);
>   
> -		p4d = p4d_offset(pgd, addr);
>   		p4d_k = p4d_offset(pgd_k, addr);
>   		if (!p4d_present(*p4d_k))
>   			goto no_context;
>   
> -		pud = pud_offset(p4d, addr);
>   		pud_k = pud_offset(p4d_k, addr);
>   		if (!pud_present(*pud_k))
>   			goto no_context;
> @@ -248,11 +246,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
>   		 * Since the vmalloc area is global, it is unnecessary
>   		 * to copy individual PTEs
>   		 */
> -		pmd = pmd_offset(pud, addr);
>   		pmd_k = pmd_offset(pud_k, addr);
>   		if (!pmd_present(*pmd_k))
>   			goto no_context;
> -		set_pmd(pmd, *pmd_k);
>   
>   		/*
>   		 * Make sure the actual PTE exists as well to
> 

Better late than never: I do agree with this patch, once the PGD is 
copied into the user page table, it "comes with all its mappings" so 
there is no need to copy PMD, the only thing left to do is to make sure 
the mapping does exists in the kernel page table.

So feel free to add:

Reviewed-by: Alexandre Ghiti <alex@...ti.fr>
Tested-by: Alexandre Ghiti <alex@...ti.fr>

Thanks,

Alex