[PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Kefeng Wang]

This reverts commit 602946ec2f90d5bd965857753880db29d2d9a1e9.

If CONFIG_HIGHMEM enabled, highmem will be disappeared with max_mapnr
set to max_low_pfn, see mem_init(), 

  for (pfn = highmem_mapnr; pfn < max_mapnr; ++pfn) {
        ...
	free_highmem_page();
  }

Revert it and will fix virt_addr_valid() check in the next patch.

Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Fixes: 602946ec2f90 ("powerpc: Set max_mapnr correctly")
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
v4:
- new patch
 arch/powerpc/mm/mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 8e301cd8925b..4d221d033804 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -255,7 +255,7 @@ void __init mem_init(void)
 #endif
 
 	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
-	set_max_mapnr(max_low_pfn);
+	set_max_mapnr(max_pfn);
 
 	kasan_late_init();
 
-- 
2.26.2

[PATCH v4 2/2] powerpc: Fix virt_addr_valid() check

[Kefeng Wang]

When run ethtool eth0 on PowerPC64, the BUG occurred,

  usercopy: Kernel memory exposure attempt detected from SLUB object not in SLUB page?! (offset 0, size 1048)!
  kernel BUG at mm/usercopy.c:99
  ...
  usercopy_abort+0x64/0xa0 (unreliable)
  __check_heap_object+0x168/0x190
  __check_object_size+0x1a0/0x200
  dev_ethtool+0x2494/0x2b20
  dev_ioctl+0x5d0/0x770
  sock_do_ioctl+0xf0/0x1d0
  sock_ioctl+0x3ec/0x5a0
  __se_sys_ioctl+0xf0/0x160
  system_call_exception+0xfc/0x1f0
  system_call_common+0xf8/0x200

The code shows below,

  data = vzalloc(array_size(gstrings.len, ETH_GSTRING_LEN));
  copy_to_user(useraddr, data, gstrings.len * ETH_GSTRING_LEN))

The data is alloced by vmalloc(), virt_addr_valid(ptr) will return true
on PowerPC64, which leads to the panic.

As commit 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va
and __pa addresses") does, make sure the virt addr above PAGE_OFFSET in
the virt_addr_valid() for PowerPC64, also add upper limit check to make
sure the virt is below high_memory.

Meanwhile, for PowerPC32 PAGE_OFFSET is the virtual address of the start
of lowmem, high_memory is the upper low virtual address, the check is
suitable for PowerPC32, this will fix the issue mentioned in commit
602946ec2f90 ("powerpc: Set max_mapnr correctly") too.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
v4:
- add upper limit check
v3:
- update changelog and remove a redundant cast
 arch/powerpc/include/asm/page.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 254687258f42..7a1ba27a7285 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -132,7 +132,11 @@ 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;			\
+	_addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory &&	\
+	pfn_valid(virt_to_pfn(_addr));					\
+})
 
 /*
  * On Book-E parts we need __va to parse the device tree and we can't
-- 
2.26.2

Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Kefeng Wang]

Hi maintainers，

I saw the patches has been reviewed[1], could they be merged?

Many thanks.

[1] https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=286464

On 2022/2/16 20:11, Kefeng Wang wrote:
> This reverts commit 602946ec2f90d5bd965857753880db29d2d9a1e9.
>
> If CONFIG_HIGHMEM enabled, highmem will be disappeared with max_mapnr
> set to max_low_pfn, see mem_init(),
>
>    for (pfn = highmem_mapnr; pfn < max_mapnr; ++pfn) {
>          ...
> 	free_highmem_page();
>    }
>
> Revert it and will fix virt_addr_valid() check in the next patch.
>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Fixes: 602946ec2f90 ("powerpc: Set max_mapnr correctly")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> v4:
> - new patch
>   arch/powerpc/mm/mem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 8e301cd8925b..4d221d033804 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -255,7 +255,7 @@ void __init mem_init(void)
>   #endif
>   
>   	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
> -	set_max_mapnr(max_low_pfn);
> +	set_max_mapnr(max_pfn);
>   
>   	kasan_late_init();
>   

Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Christophe Leroy]

Hi,

Le 26/03/2022 à 08:55, Kefeng Wang a écrit :
> Hi maintainers，
> 
> I saw the patches has been reviewed[1], could they be merged?

Thinking about it once more, I think the patches should go in reverse 
order. Patch 2 should go first and patch 1 should go after.

Otherwise, once patch 1 is applied and patch 2 is not applied yet, 
virt_addr_valid() doesn't work anymore.

Christophe

> 
> Many thanks.
> 
> [1] https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=286464
> 
> On 2022/2/16 20:11, Kefeng Wang wrote:
>> This reverts commit 602946ec2f90d5bd965857753880db29d2d9a1e9.
>>
>> If CONFIG_HIGHMEM enabled, highmem will be disappeared with max_mapnr
>> set to max_low_pfn, see mem_init(),
>>
>>    for (pfn = highmem_mapnr; pfn < max_mapnr; ++pfn) {
>>          ...
>>     free_highmem_page();
>>    }
>>
>> Revert it and will fix virt_addr_valid() check in the next patch.
>>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Fixes: 602946ec2f90 ("powerpc: Set max_mapnr correctly")
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>> v4:
>> - new patch
>>   arch/powerpc/mm/mem.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index 8e301cd8925b..4d221d033804 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -255,7 +255,7 @@ void __init mem_init(void)
>>   #endif
>>       high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
>> -    set_max_mapnr(max_low_pfn);
>> +    set_max_mapnr(max_pfn);
>>       kasan_late_init();

Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Kefeng Wang]

On 2022/3/28 22:12, Christophe Leroy wrote:
> Hi,
>
> Le 26/03/2022 à 08:55, Kefeng Wang a écrit :
>> Hi maintainers，
>>
>> I saw the patches has been reviewed[1], could they be merged?
> Thinking about it once more, I think the patches should go in reverse
> order. Patch 2 should go first and patch 1 should go after.
>
> Otherwise, once patch 1 is applied and patch 2 is not applied yet,
> virt_addr_valid() doesn't work anymore.
Should I resend them or could the maintainer reverse order when merging 
them?
>
> Christophe
>
>> Many thanks.
>>
>> [1] https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=286464
>>

Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Michael Ellerman]

Kefeng Wang <wangkefeng.wang@huawei.com> writes:
> On 2022/3/28 22:12, Christophe Leroy wrote:
>> Hi,
>>
>> Le 26/03/2022 à 08:55, Kefeng Wang a écrit :
>>> Hi maintainers，
>>>
>>> I saw the patches has been reviewed[1], could they be merged?
>> Thinking about it once more, I think the patches should go in reverse
>> order. Patch 2 should go first and patch 1 should go after.
>>
>> Otherwise, once patch 1 is applied and patch 2 is not applied yet,
>> virt_addr_valid() doesn't work anymore.
>
> Should I resend them or could the maintainer reverse order when merging 
> them?

I'll reverse them. I've found some breakage in other code while testing
this, so I'll fix that up first before merging these.

In patch 2 you didn't say what hardware you hit this on, what CPU does
your system have?

cheers

Re: [PATCH v4 1/2] Revert "powerpc: Set max_mapnr correctly"

[Kefeng Wang]

On 2022/4/4 20:31, Michael Ellerman wrote:
> Kefeng Wang <wangkefeng.wang@huawei.com> writes:
>> On 2022/3/28 22:12, Christophe Leroy wrote:
>>> Hi,
>>>
>>> Le 26/03/2022 à 08:55, Kefeng Wang a écrit :
>>>> Hi maintainers，
>>>>
>>>> I saw the patches has been reviewed[1], could they be merged?
>>> Thinking about it once more, I think the patches should go in reverse
>>> order. Patch 2 should go first and patch 1 should go after.
>>>
>>> Otherwise, once patch 1 is applied and patch 2 is not applied yet,
>>> virt_addr_valid() doesn't work anymore.
>> Should I resend them or could the maintainer reverse order when merging
>> them?
> I'll reverse them. I've found some breakage in other code while testing
> this, so I'll fix that up first before merging these.
Thanks.
>
> In patch 2 you didn't say what hardware you hit this on, what CPU does
> your system have?

CPU e5500  from fsl,P5040DS.

>
> cheers
> .