Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Tue, 6 Apr 2021 09:19:52 +0200 | Subject | Re: [PATCH v3 3/5] RISC-V: Improve init_resources |
| |
Hi Nick,
Thanks for your patch!
On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis <mick@ics.forth.gr> wrote: > * Kernel region is always present and we know where it is, no > need to look for it inside the loop, just ignore it like the > rest of the reserved regions within system's memory. > > * Don't call memblock_free inside the loop, if called it'll split > the region of pre-allocated resources in two parts, messing things > up, just re-use the previous pre-allocated resource and free any > unused resources after both loops finish. > > * memblock_alloc may add a region when called, so increase the > number of pre-allocated regions by one to be on the safe side > (reported and patched by Geert Uytterhoeven) > > Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Where does this SoB come from?
> Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
> --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c
> @@ -129,53 +139,42 @@ static void __init init_resources(void) > struct resource *res = NULL; > struct resource *mem_res = NULL; > size_t mem_res_sz = 0; > - int ret = 0, i = 0; > - > - code_res.start = __pa_symbol(_text); > - code_res.end = __pa_symbol(_etext) - 1; > - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - rodata_res.start = __pa_symbol(__start_rodata); > - rodata_res.end = __pa_symbol(__end_rodata) - 1; > - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > - > - data_res.start = __pa_symbol(_data); > - data_res.end = __pa_symbol(_edata) - 1; > - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + int num_resources = 0, res_idx = 0; > + int ret = 0; > > - bss_res.start = __pa_symbol(__bss_start); > - bss_res.end = __pa_symbol(__bss_stop) - 1; > - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; > + /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ > + num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1; > + res_idx = num_resources - 1; > > - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * sizeof(*mem_res);
Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix out-of-bounds accesses in init_resources()") (from v5.12-rc4) into your patch. Why? This means your patch does not apply against upstream.
> + mem_res_sz = num_resources * sizeof(*mem_res); > mem_res = memblock_alloc(mem_res_sz, SMP_CACHE_BYTES); > if (!mem_res) > panic("%s: Failed to allocate %zu bytes\n", __func__, mem_res_sz);
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |