linux-kernel - Re: [PATCH V2] mips: kernel: fix detect_memory

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]

Date: Sat, 29 Jun 2024 13:19:01 +0800
From: Shiji Yang <yangshiji66@...look.com>
To: tsbogend@...ha.franken.de
Cc: arnd@...db.de,
	bhe@...hat.com,
	fancer.lancer@...il.com,
	gregkh@...uxfoundation.org,
	javierm@...hat.com,
	jiaxun.yang@...goat.com,
	khalid@...ehiking.org,
	linux-kernel@...r.kernel.org,
	linux-mips@...r.kernel.org,
	namiltd@...oo.com,
	yangshiji66@...look.com
Subject: Re: [PATCH V2] mips: kernel: fix detect_memory_region() function

On Tue, 25 Jun 2024 09:46:34 +0200, Thomas Bogendoerfer wrote:
>>  	for (size = sz_min; size < sz_max; size <<= 1) {
>> -		if (!memcmp(dm, dm + size, sizeof(detect_magic)))
>> -			break;
>> +		__raw_writel(MIPS_MEM_TEST_PATTERN, dm);
>> +		if (__raw_readl(dm) == __raw_readl(dm + size)) {
>> +			__raw_writel(~MIPS_MEM_TEST_PATTERN, dm);
>> +			if (__raw_readl(dm) == __raw_readl(dm + size))
>> +				break;
>> +		}
>
>you are testing memory, so just use pointers. __raw_readl and __raw_writel
>are for io regions (which should be ioremppaed before usage).

Fixed in v3:
https://lore.kernel.org/linux-mips/TYCP286MB0895F65439037ED134FEA7DDBCD12@TYCP286MB0895.JPNP286.PROD.OUTLOOK.COM/


>
>> The detect_memory_region() has been broken on 6.6 kernel[1]. This
>> patch fixes it by:
>> 1. Do not use memcmp() on unallocated memory, as the new introduced
>>    fortify dynamic object size check[2] will return unexpected result.
>
>hmm, so there should a new way for doing memory probing without
>triggering this fortify check. How do other platforms deal with this ?

I guess __builtin_memcmp() should work. But this patch also fixes a
potential wrong memory size issue[1] by fliping magic number and
double checking it. And there is no need to ues memcmp() to check
an u32 variable.

It seems that other ARCHs directly read some registers to judge
the memory size. However, the theory in mips ARCH is checking the
overlapping memory address.

>
>> 2. Use a fixed pattern instead of a random function pointer as the
>>    magic value.
>
>why is this better ?

Just engineering experience. Byte 0xaa/0x55 has the largest information
entropy and is widely used in memory testing. Most codes in v2 patch
are copied from 'arch/mips/ralink/mt7621.c'.

>
>Thomas.
>

[1] https://github.com/openwrt/openwrt/pull/14282


Regards,
Shiji Yang