linux-kernel - Re: [PATCH] MIPS: reserve the memblock right after the kernel

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

Date:   Fri, 13 Nov 2020 10:17:55 +0100
From:   Alexander Sverdlin <alexander.sverdlin@...ia.com>
To:     Serge Semin <fancer.lancer@...il.com>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        linux-mips@...r.kernel.org, Paul Burton <paulburton@...nel.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] MIPS: reserve the memblock right after the kernel

Hello Serge, Thomas,

On 11/11/2020 15:52, Serge Semin wrote:
>>> Could you send a patch, which removes check_kernel_section_mem completly ?

finally I think you are right and this would be the right way.

>> this will expose one issue:
>> platforms usually do it in a sane way, like it was done last 15 years, namely
>> add kernel image without non-complete pages on the boundaries.
>> This will lead to the situation, that request_resource() will fail at least
>> for .bss section of the kernel and it will not be properly displayed under
>> /proc/iomem (and probably same problem will appear, which initially motivated
>> the creation of check_kernel_section_mem()).
> 
> Are you saying that some old platforms rely on the
> check_kernel_section_mem() method adding the memory occupied by the
> kernel to the system? If so, do you have an example of such?

Initially I was confused why the below patch didn't solve the issue on Octeon:

@@ -532,8 +532,8 @@ static void __init request_crashkernel(struct resource *res)
 
 static void __init check_kernel_sections_mem(void)
 {
-       phys_addr_t start = PFN_PHYS(PFN_DOWN(__pa_symbol(&_text)));
-       phys_addr_t size = PFN_PHYS(PFN_UP(__pa_symbol(&_end))) - start;
+       phys_addr_t start = __pa_symbol(&_text);
+       phys_addr_t size = __pa_symbol(&_end) - start;

... and finally I understood, that the reason was in fact that I tested on Linux
v5.4, which still had this code to reserve RAM resources:

        for_each_memblock(memory, region) {                                                                                                                                                                        
                phys_addr_t start = PFN_PHYS(memblock_region_memory_base_pfn(region));                                                                                                                             
                phys_addr_t end = PFN_PHYS(memblock_region_memory_end_pfn(region)) - 1;                                                                                                                            
                struct resource *res;                                                                                                                                                                              
                                                                                                                                                                                                                   
                ...
	
                res->start = start;                                                                                                                                                                                
                res->end = end;                                                                                                                                                                                    
                res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;                                                                                                                                              
                res->name = "System RAM";                                                                                                                                                                          
                                                                                                                                                                                                                   
                request_resource(&iomem_resource, res);                                                                                                                                                            
 
so I suppose that's where this evil truncation happened. Nowdays this is different
and I believe we can try to remove check_kernel_sections_mem() completely and
this will solve the memory corruption on Octeon.

> So IMHO what could be the best conclusion in the framework of this patch:
> 1) As Thomas said any platform-specific reservation should be done in the
> platform-specific code. That means if octeon needs some memory behind
> the kernel being reserved, then it should be done for example in
> prom_init().
> 2) The check_kernel_sections_mem() method can be removed. But it
> should be done carefully. We at least need to try to find all the
> platforms, which rely on its functionality.

Thanks for looking into this! I agree with your analysis, I'll try to rework,
removing check_kernel_sections_mem().

-- 
Best regards,
Alexander Sverdlin.