| 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
| ||
|
Message-ID: <244f4b37-2db6-6f9e-b5e2-3ccd6c3136ba@arm.com>
Date: Wed, 25 Aug 2021 11:33:41 +0100
From: Robin Murphy <robin.murphy@....com>
To: Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>
Cc: David Hildenbrand <david@...hat.com>,
Mike Rapoport <rppt@...nel.org>,
Alex Bee <knaerzche@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Anshuman Khandual <anshuman.khandual@....com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-mm@...ck.org,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
Christoph Hellwig <hch@....de>
Subject: Re: [BUG 5.14] arm64/mm: dma memory mapping fails (in some cases)
On 2021-08-25 11:28, Will Deacon wrote:
> On Wed, Aug 25, 2021 at 11:20:46AM +0100, Catalin Marinas wrote:
>> Given how later we are in the -rc cycle, I suggest we revert Anshuman's
>> commit 16c9afc77660 ("arm64/mm: drop HAVE_ARCH_PFN_VALID") and try to
>> assess the implications in 5.15 (the patch doesn't seem to have the
>> arm64 maintainers' ack anyway ;)).
>
> I'll stick the revert (below) into kernelci now so we can get some coverage
> in case it breaks something else.
>
> Will
>
> --->8
>
> From e97ba0e39e486c20d8f76f3e632e4b7d933602cd Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@...nel.org>
> Date: Wed, 25 Aug 2021 11:10:07 +0100
> Subject: [PATCH] Revert "arm64/mm: drop HAVE_ARCH_PFN_VALID"
>
> This reverts commit 16c9afc776608324ca71c0bc354987bab532f51d.
>
> Alex Bee reports a regression in 5.14 on their RK3328 SoC when
> configuring the PL330 DMA controller:
>
> | ------------[ cut here ]------------
> | WARNING: CPU: 2 PID: 373 at kernel/dma/mapping.c:235 dma_map_resource+0x68/0xc0
> | Modules linked in: spi_rockchip(+) fuse
> | CPU: 2 PID: 373 Comm: systemd-udevd Not tainted 5.14.0-rc7 #1
> | Hardware name: Pine64 Rock64 (DT)
> | pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> | pc : dma_map_resource+0x68/0xc0
> | lr : pl330_prep_slave_fifo+0x78/0xd0
>
> This appears to be because dma_map_resource() is being called for a
> physical address which does not correspond to a memory address yet does
> have a valid 'struct page' due to the way in which the vmemmap is
> constructed.
>
> Prior to 16c9afc77660 ("arm64/mm: drop HAVE_ARCH_PFN_VALID"), the arm64
> implementation of pfn_valid() called memblock_is_memory() to return
> 'false' for such regions and the DMA mapping request would proceed.
> However, now that we are using the generic implementation where only the
> presence of the memory map entry is considered, we return 'true' and
> erroneously fail with DMA_MAPPING_ERROR because we identify the region
> as DRAM.
>
> Although fixing this in the DMA mapping code is arguably the right fix,
> it is a risky, cross-architecture change at this stage in the cycle. So
> just revert arm64 back to its old pfn_valid() implementation for v5.14.
TBH the offending warning is only meant to be a quick sanity check, so I
don't think there should be much impact to changing the DMA code; it's
just a question of figuring out what change to make. I'm digging in now...
Thanks,
Robin.
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Robin Murphy <robin.murphy@....com>
> Cc: Mike Rapoport <rppt@...nel.org>
> Cc: Anshuman Khandual <anshuman.khandual@....com>
> Cc: Christoph Hellwig <hch@....de>
> Reported-by: Alex Bee <knaerzche@...il.com>
> Link: https://lore.kernel.org/r/d3a3c828-b777-faf8-e901-904995688437@gmail.com
> Signed-off-by: Will Deacon <will@...nel.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/page.h | 1 +
> arch/arm64/mm/init.c | 37 +++++++++++++++++++++++++++++++++++
> include/linux/mmzone.h | 9 ---------
> 4 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fdcd54d39c1e..62c3c1d2190f 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -156,6 +156,7 @@ config ARM64
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_MMAP_RND_BITS
> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
> + select HAVE_ARCH_PFN_VALID
> select HAVE_ARCH_PREL32_RELOCATIONS
> select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
> select HAVE_ARCH_SECCOMP_FILTER
> diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
> index 993a27ea6f54..f98c91bbd7c1 100644
> --- a/arch/arm64/include/asm/page.h
> +++ b/arch/arm64/include/asm/page.h
> @@ -41,6 +41,7 @@ void tag_clear_highpage(struct page *to);
>
> typedef struct page *pgtable_t;
>
> +int pfn_valid(unsigned long pfn);
> int pfn_is_map_memory(unsigned long pfn);
>
> #include <asm/memory.h>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 8490ed2917ff..1fdb7bb7c198 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -219,6 +219,43 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> free_area_init(max_zone_pfns);
> }
>
> +int pfn_valid(unsigned long pfn)
> +{
> + phys_addr_t addr = PFN_PHYS(pfn);
> + struct mem_section *ms;
> +
> + /*
> + * Ensure the upper PAGE_SHIFT bits are clear in the
> + * pfn. Else it might lead to false positives when
> + * some of the upper bits are set, but the lower bits
> + * match a valid pfn.
> + */
> + if (PHYS_PFN(addr) != pfn)
> + return 0;
> +
> + if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> + return 0;
> +
> + ms = __pfn_to_section(pfn);
> + if (!valid_section(ms))
> + return 0;
> +
> + /*
> + * ZONE_DEVICE memory does not have the memblock entries.
> + * memblock_is_map_memory() check for ZONE_DEVICE based
> + * addresses will always fail. Even the normal hotplugged
> + * memory will never have MEMBLOCK_NOMAP flag set in their
> + * memblock entries. Skip memblock search for all non early
> + * memory sections covering all of hotplug memory including
> + * both normal and ZONE_DEVICE based.
> + */
> + if (!early_section(ms))
> + return pfn_section_valid(ms, pfn);
> +
> + return memblock_is_memory(addr);
> +}
> +EXPORT_SYMBOL(pfn_valid);
> +
> int pfn_is_map_memory(unsigned long pfn)
> {
> phys_addr_t addr = PFN_PHYS(pfn);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index fcb535560028..ee70f21a79d5 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1463,15 +1463,6 @@ static inline int pfn_valid(unsigned long pfn)
> {
> struct mem_section *ms;
>
> - /*
> - * Ensure the upper PAGE_SHIFT bits are clear in the
> - * pfn. Else it might lead to false positives when
> - * some of the upper bits are set, but the lower bits
> - * match a valid pfn.
> - */
> - if (PHYS_PFN(PFN_PHYS(pfn)) != pfn)
> - return 0;
> -
> if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
> return 0;
> ms = __nr_to_section(pfn_to_section_nr(pfn));
>
Powered by blists - more mailing lists