Re: [PATCH] Revert "mm/usercopy: Drop extra is_vmalloc_or_module() check"

[Kefeng Wang]

On 2021/12/25 19:04, Nicholas Piggin wrote:
> Excerpts from Kefeng Wang's message of December 25, 2021 12:05 pm:
...
> > > Can you try that ?
> > >
> > > #define virt_addr_valid(kaddr)	((kaddr & PAGE_OFFSET) == PAGE_OFFSET &&
> > > pfn_valid(virt_to_pfn(kaddr)))
> > I got this commit,
> >
> > commit 4dd7554a6456d124c85e0a4ad156625b71390b5c
> >
> > Author: Nicholas Piggin <npiggin@xxxxxxxxx>
> > Date:   Wed Jul 24 18:46:37 2019 +1000
> >
> >       powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses
> >
> >       Ensure __va is given a physical address below PAGE_OFFSET, and __pa is
> >       given a virtual address above PAGE_OFFSET.
> >
> > It has check the PAGE_OFFSET in __pa,  will test it and resend the
> > patch(with above warning changes).
> What did you get with this commit? Is this what causes the crash?

I mean that your patch does the check to make sure the virt addr should 
above PAGE_OFFSET,

and we can add the check in the virt_addr_valid too.

> riscv for example with flatmem also relies on pfn_valid to do the right
> thing, so as far as I can see the check should exclude vmalloc addresses
> and it's just a matter of virt_addr_valid not to give virt_to_pfn an
> address < PAGE_OFFSET.
>
> If we take riscv's implementation
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 254687258f42..7713188516a6 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -132,7 +132,10 @@ static inline bool pfn_valid(unsigned long pfn)
>   #define virt_to_page(kaddr)    pfn_to_page(virt_to_pfn(kaddr))
>   #define pfn_to_kaddr(pfn)      __va((pfn) << PAGE_SHIFT)
>   
> -#define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr))
> +#define virt_addr_valid(vaddr) ({                                      \
> +       unsigned long _addr = (unsigned long)vaddr;                     \
> +       (unsigned long)(_addr) >= PAGE_OFFSET && pfn_valid(virt_to_pfn(_addr)); \
> +})
Yes, I send a new v2  with this change, thanks
>   
>   /*
>    * On Book-E parts we need __va to parse the device tree and we can't
>
> .