linux-kernel - Re: ioremap() called early from pnv_pci_init_ioda

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]
Message-Id: <1589013450.02gfmpktnp.astroid@bobo.none>
Date:   Sat, 09 May 2020 18:38:36 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Qian Cai <cai@....pw>, Christophe Leroy <christophe.leroy@....fr>,
        Michael Ellerman <mpe@...erman.id.au>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: ioremap() called early from pnv_pci_init_ioda_phb()

Excerpts from Qian Cai's message of May 9, 2020 3:41 am:
> 
> 
>> On May 8, 2020, at 10:39 AM, Qian Cai <cai@....pw> wrote:
>> 
>> Booting POWER9 PowerNV has this message,
>> 
>> "ioremap() called early from pnv_pci_init_ioda_phb+0x420/0xdfc. Use early_ioremap() instead”
>> 
>> but use the patch below will result in leaks because it will never call early_iounmap() anywhere. However, it looks me it was by design that phb->regs mapping would be there forever where it would be used in pnv_ioda_get_inval_reg(), so is just that check_early_ioremap_leak() initcall too strong?
>> 
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -36,6 +36,7 @@
>> #include <asm/firmware.h>
>> #include <asm/pnv-pci.h>
>> #include <asm/mmzone.h>
>> +#include <asm/early_ioremap.h>
>> 
>> #include <misc/cxl-base.h>
>> 
>> @@ -3827,7 +3828,7 @@ static void __init pnv_pci_init_ioda_phb(struct device_node *np,
>>        /* Get registers */
>>        if (!of_address_to_resource(np, 0, &r)) {
>>                phb->regs_phys = r.start;
>> -               phb->regs = ioremap(r.start, resource_size(&r));
>> +               phb->regs = early_ioremap(r.start, resource_size(&r));
>>                if (phb->regs == NULL)
>>                        pr_err("  Failed to map registers !\n”);
> 
> This will also trigger a panic with debugfs reads, so isn’t that this commit bogus at least for powerpc64?

Your patch to use early_ioremap is faulting? I wonder why?

Thanks,
Nick

> 
> d538aadc2718 (“powerpc/ioremap: warn on early use of ioremap()")
> 
> 11017.617022][T122068] Faulting instruction address: 0xc0000000000db564
> [11017.617257][T122066] Faulting instruction address: 0xc0000000000db564
> [11017.617950][T122073] Faulting instruction address: 0xc0000000000db564
> [11017.618888][T122064] BUG: Unable to handle kernel data access on read at 0xffffffffffe20e10
> [11017.618935][T122064] Faulting instruction address: 0xc0000000000db564
> [11017.737996][T122072] 
> [11017.738010][T122073] Oops: Kernel access of bad area, sig: 11 [#2]
> [11017.738024][T122073] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
> [11017.738051][T122073] Modules linked in: brd ext4 crc16 mbcache jbd2 loop kvm_hv kvm ip_tables x_tables xfs sd_mod bnx2x ahci libahci tg3 mdio libata libphy firmware_class dm_mirror dm_region_hash dm_log dm_mod
> [11017.738110][T122073] CPU: 108 PID: 122073 Comm: read_all Tainted: G      D W         5.7.0-rc4-next-20200508+ #4
> [11017.738138][T122073] NIP:  c0000000000db564 LR: c00000000056f660 CTR: c0000000000db550
> [11017.738173][T122073] REGS: c000000374f6f980 TRAP: 0380   Tainted: G      D W          (5.7.0-rc4-next-20200508+)
> [11017.738234][T122073] MSR:  9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE>  CR: 22002282  XER: 20040000
> [11017.738278][T122073] CFAR: c00000000056f65c IRQMASK: 0 
> [11017.738278][T122073] GPR00: c00000000056f660 c000000374f6fc10 c000000001689400 c000201ffc41aa00 
> [11017.738278][T122073] GPR04: c000000374f6fc70 0000000000000000 0000000000000000 0000000000000001 
> [11017.738278][T122073] GPR08: 0000000000000000 ffffffffffe20000 0000000000000000 c0000008ee380080 
> [11017.738278][T122073] GPR12: c0000000000db550 c000201fff671280 0000000000000000 0000000000000000 
> [11017.738278][T122073] GPR16: 0000000000000002 0000000010040800 000000001001ccd8 000000001001cc80 
> [11017.738278][T122073] GPR20: 000000001001cc98 000000001001ccc8 000000001001cca8 000000001001cb48 
> [11017.738278][T122073] GPR24: 0000000000000000 0000000000000000 00000000000003ff 00007fffebb67390 
> [11017.738278][T122073] GPR28: c000000374f6fd90 c000200c0c6a7550 0000000000000000 c000200c0c6a7500 
> [11017.738542][T122073] NIP [c0000000000db564] pnv_eeh_dbgfs_get_inbB+0x14/0x30
> [11017.738579][T122073] LR [c00000000056f660] simple_attr_read+0xa0/0x180
> [11017.738613][T122073] Call Trace:
> [11017.738645][T122073] [c000000374f6fc10] [c00000000056f630] simple_attr_read+0x70/0x180 (unreliable)
> [11017.738672][T122073] [c000000374f6fcb0] [c00000000064a2e0] full_proxy_read+0x90/0xe0
> [11017.738686][T122073] [c000000374f6fd00] [c00000000051fe0c] __vfs_read+0x3c/0x70
> [11017.738722][T122073] [c000000374f6fd20] [c00000000051feec] vfs_read+0xac/0x170
> [11017.738757][T122073] [c000000374f6fd70] [c00000000052034c] ksys_read+0x7c/0x140
> [11017.738818][T122073] [c000000374f6fdc0] [c000000000038af4] system_call_exception+0x114/0x1e0
> [11017.738867][T122073] [c000000374f6fe20] [c00000000000c8f0] system_call_common+0xf0/0x278
> [11017.738916][T122073] Instruction dump:
> [11017.738948][T122073] 7c0004ac f9490d10 a14d0c78 38600000 b14d0c7a 4e800020 60000000 7c0802a6 
> [11017.739001][T122073] 60000000 e9230278 e9290028 7c0004ac <e9290e10> 0c090000 4c00012c 38600000