From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55458C04EB8 for ; Mon, 10 Dec 2018 14:36:23 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 247A72084E for ; Mon, 10 Dec 2018 14:36:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="HlUUrC2/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 247A72084E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KND3E8PaQJEpHub/eVjDTCBTiYBiG3oOEvj1YPNAFgs=; b=HlUUrC2/BCZxjDB/UnE16X8jz E19TIISRQmmWF7rLoq8ESTx1Jt6U2eBipewoznT2cYhdwSXtY54upQjqDdSDi6Jocz8gp/2/az/QZ jVTZenu2E+9DTV6eX+lN4IY+54vlibjtCG/V6dRpFjrLRzKiUUAYvn2SQPZiSJ3IPsrM5hZCVEsRQ w2hmndRxFmfCp66Qes4kug4m1ZQ8Cbj9+4dISXLHW9OURnjzEXZ6d9xrc4VhipR2bFzQacmEatTDL apAPoT9VMpjvcpV6O8jfxhYLhYK7oz6v84OsQBM9qf0TEDFOQDNTNAV9+WPok5pOIAIX9oaBIlSoQ 7GOcldB3Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gWMfS-0003Th-Se; Mon, 10 Dec 2018 14:36:18 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gWMfO-0003Sk-MU for linux-arm-kernel@lists.infradead.org; Mon, 10 Dec 2018 14:36:16 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0A2461596; Mon, 10 Dec 2018 06:36:04 -0800 (PST) Received: from [10.1.196.75] (e110467-lin.cambridge.arm.com [10.1.196.75]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id ECEE43F575; Mon, 10 Dec 2018 06:35:56 -0800 (PST) Subject: Re: [PATCH] mm/zsmalloc.c: Fix zsmalloc 32-bit PAE support To: Rafael David Tinoco , Russell King References: <20181210142105.6750-1-rafael.tinoco@linaro.org> From: Robin Murphy Message-ID: <4da655ec-a1ac-c524-1eb4-5cbd18b265ef@arm.com> Date: Mon, 10 Dec 2018 14:35:55 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181210142105.6750-1-rafael.tinoco@linaro.org> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20181210_063614_750375_E7F43227 X-CRM114-Status: GOOD ( 27.64 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Rich Felker , linux-ia64@vger.kernel.org, Sergey Senozhatsky , linux-sh@vger.kernel.org, James Hogan , Heiko Carstens , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Khalid Aziz , Paul Mackerras , "H . Peter Anvin" , sparclinux@vger.kernel.org, Catalin Marinas , linux-s390@vger.kernel.org, Yoshinori Sato , Michael Ellerman , x86@kernel.org, Will Deacon , Ingo Molnar , Benjamin Herrenschmidt , Anthony Yznaga , Nitin Gupta , Fenghua Yu , Joerg Roedel , Vasily Gorbik , Ram Pai , Nicholas Piggin , Borislav Petkov , Andy Lutomirski , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Juergen Gross , Tony Luck , Jiri Kosina , linux-mips@vger.kernel.org, Ralf Baechle , Minchan Kim , Paul Burton , Christophe Leroy , "Aneesh Kumar K . V" , Martin Schwidefsky , linuxppc-dev@lists.ozlabs.org, "David S . Miller" , "Kirill A . Shutemov" Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 10/12/2018 14:21, Rafael David Tinoco wrote: > On 32-bit systems, zsmalloc uses HIGHMEM and, when PAE is enabled, the > physical frame number might be so big that zsmalloc obj encoding (to > location) will break, causing: > > BUG: KASAN: null-ptr-deref in zs_map_object+0xa4/0x2bc > Read of size 4 at addr 00000000 by task mkfs.ext4/623 > CPU: 2 PID: 623 Comm: mkfs.ext4 Not tainted 4.19.0-rc8-00017-g8239bc6d3307-dirty #15 > Hardware name: Generic DT based system > [] (unwind_backtrace) from [] (show_stack+0x20/0x24) > [] (show_stack) from [] (dump_stack+0xbc/0xe8) > [] (dump_stack) from [] (kasan_report+0x248/0x390) > [] (kasan_report) from [] (__asan_load4+0x78/0xb4) > [] (__asan_load4) from [] (zs_map_object+0xa4/0x2bc) > [] (zs_map_object) from [] (zram_bvec_rw.constprop.2+0x324/0x8e4 [zram]) > [] (zram_bvec_rw.constprop.2 [zram]) from [] (zram_make_request+0x234/0x46c [zram]) > [] (zram_make_request [zram]) from [] (generic_make_request+0x304/0x63c) > [] (generic_make_request) from [] (submit_bio+0x4c/0x1c8) > [] (submit_bio) from [] (submit_bh_wbc.constprop.15+0x238/0x26c) > [] (submit_bh_wbc.constprop.15) from [] (__block_write_full_page+0x524/0x76c) > [] (__block_write_full_page) from [] (block_write_full_page+0x1bc/0x1d4) > [] (block_write_full_page) from [] (blkdev_writepage+0x24/0x28) > [] (blkdev_writepage) from [] (__writepage+0x44/0x78) > [] (__writepage) from [] (write_cache_pages+0x3b8/0x800) > [] (write_cache_pages) from [] (generic_writepages+0x74/0xa0) > [] (generic_writepages) from [] (blkdev_writepages+0x18/0x1c) > [] (blkdev_writepages) from [] (do_writepages+0x68/0x134) > [] (do_writepages) from [] (__filemap_fdatawrite_range+0xb0/0xf4) > [] (__filemap_fdatawrite_range) from [] (file_write_and_wait_range+0x64/0xd0) > [] (file_write_and_wait_range) from [] (blkdev_fsync+0x54/0x84) > [] (blkdev_fsync) from [] (vfs_fsync_range+0x70/0xd4) > [] (vfs_fsync_range) from [] (do_fsync+0x4c/0x80) > [] (do_fsync) from [] (sys_fsync+0x1c/0x20) > [] (sys_fsync) from [] (ret_fast_syscall+0x0/0x2c) > > when trying to decode (the pfn) and map the object. > > That happens because one architecture might not re-define > MAX_PHYSMEM_BITS, like in this ARM 32-bit w/ LPAE enabled example. For > 32-bit systems, if not re-defined, MAX_POSSIBLE_PHYSMEM_BITS will > default to BITS_PER_LONG (32) in most cases, and, with PAE enabled, > _PFN_BITS might be wrong: which may cause obj variable to overflow if > frame number is in HIGHMEM and referencing a page above the 4GB > watermark. > > commit 6e00ec00b1a7 ("staging: zsmalloc: calculate MAX_PHYSMEM_BITS if > not defined") realized MAX_PHYSMEM_BITS depended on SPARSEMEM headers > and "fixed" it by calculating it using BITS_PER_LONG if SPARSEMEM wasn't > used, like in the example given above. > > Systems with potential for PAE exist for a long time and assuming > BITS_PER_LONG seems inadequate. Defining MAX_PHYSMEM_BITS looks better, > however it is NOT a constant anymore for x86. > > SO, instead, MAX_POSSIBLE_PHYSMEM_BITS should be defined by every > architecture using zsmalloc, together with a sanity check for > MAX_POSSIBLE_PHYSMEM_BITS being too big on 32-bit systems. > > Link: https://bugs.linaro.org/show_bug.cgi?id=3765#c17 > Signed-off-by: Rafael David Tinoco > --- > arch/arm/include/asm/pgtable-2level-types.h | 2 ++ > arch/arm/include/asm/pgtable-3level-types.h | 2 ++ > arch/arm64/include/asm/pgtable-types.h | 2 ++ > arch/ia64/include/asm/page.h | 2 ++ > arch/mips/include/asm/page.h | 2 ++ > arch/powerpc/include/asm/mmu.h | 2 ++ > arch/s390/include/asm/page.h | 2 ++ > arch/sh/include/asm/page.h | 2 ++ > arch/sparc/include/asm/page_32.h | 2 ++ > arch/sparc/include/asm/page_64.h | 2 ++ > arch/x86/include/asm/pgtable-2level_types.h | 2 ++ > arch/x86/include/asm/pgtable-3level_types.h | 3 +- > arch/x86/include/asm/pgtable_64_types.h | 4 +-- > mm/zsmalloc.c | 35 +++++++++++---------- > 14 files changed, 45 insertions(+), 19 deletions(-) > > diff --git a/arch/arm/include/asm/pgtable-2level-types.h b/arch/arm/include/asm/pgtable-2level-types.h > index 66cb5b0e89c5..552dba411324 100644 > --- a/arch/arm/include/asm/pgtable-2level-types.h > +++ b/arch/arm/include/asm/pgtable-2level-types.h > @@ -64,4 +64,6 @@ typedef pteval_t pgprot_t; > > #endif /* STRICT_MM_TYPECHECKS */ > > +#define MAX_POSSIBLE_PHYSMEM_BITS 32 > + > #endif /* _ASM_PGTABLE_2LEVEL_TYPES_H */ > diff --git a/arch/arm/include/asm/pgtable-3level-types.h b/arch/arm/include/asm/pgtable-3level-types.h > index 921aa30259c4..664c39e6517c 100644 > --- a/arch/arm/include/asm/pgtable-3level-types.h > +++ b/arch/arm/include/asm/pgtable-3level-types.h > @@ -67,4 +67,6 @@ typedef pteval_t pgprot_t; > > #endif /* STRICT_MM_TYPECHECKS */ > > +#define MAX_POSSIBLE_PHYSMEM_BITS 36 Nit: with LPAE, physical addresses go up to 40 bits, not just 36. Robin. > + > #endif /* _ASM_PGTABLE_3LEVEL_TYPES_H */ > diff --git a/arch/arm64/include/asm/pgtable-types.h b/arch/arm64/include/asm/pgtable-types.h > index 345a072b5856..45c3834eb4c8 100644 > --- a/arch/arm64/include/asm/pgtable-types.h > +++ b/arch/arm64/include/asm/pgtable-types.h > @@ -64,4 +64,6 @@ typedef struct { pteval_t pgprot; } pgprot_t; > #include > #endif > > +#define MAX_POSSIBLE_PHYSMEM_BITS CONFIG_ARM64_PA_BITS > + > #endif /* __ASM_PGTABLE_TYPES_H */ > diff --git a/arch/ia64/include/asm/page.h b/arch/ia64/include/asm/page.h > index 5798bd2b462c..a3e055979e46 100644 > --- a/arch/ia64/include/asm/page.h > +++ b/arch/ia64/include/asm/page.h > @@ -235,4 +235,6 @@ get_order (unsigned long size) > > #define __HAVE_ARCH_GATE_AREA 1 > > +#define MAX_POSSIBLE_PHYSMEM_BITS 50 > + > #endif /* _ASM_IA64_PAGE_H */ > diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h > index e8cc328fce2d..f6a5dea1a66c 100644 > --- a/arch/mips/include/asm/page.h > +++ b/arch/mips/include/asm/page.h > @@ -263,4 +263,6 @@ extern int __virt_addr_valid(const volatile void *kaddr); > #include > #include > > +#define MAX_POSSIBLE_PHYSMEM_BITS 48 > + > #endif /* _ASM_PAGE_H */ > diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h > index eb20eb3b8fb0..2ebc1d2d9a5c 100644 > --- a/arch/powerpc/include/asm/mmu.h > +++ b/arch/powerpc/include/asm/mmu.h > @@ -324,6 +324,8 @@ static inline u16 get_mm_addr_key(struct mm_struct *mm, unsigned long address) > #define MAX_PHYSMEM_BITS 46 > #endif > > +#define MAX_POSSIBLE_PHYSMEM_BITS MAX_PHYSMEM_BITS > + > #ifdef CONFIG_PPC_BOOK3S_64 > #include > #else /* CONFIG_PPC_BOOK3S_64 */ > diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h > index a4d38092530a..8abec1461bf7 100644 > --- a/arch/s390/include/asm/page.h > +++ b/arch/s390/include/asm/page.h > @@ -180,4 +180,6 @@ static inline int devmem_is_allowed(unsigned long pfn) > #include > #include > > +#define MAX_POSSIBLE_PHYSMEM_BITS CONFIG_MAX_PHYSMEM_BITS > + > #endif /* _S390_PAGE_H */ > diff --git a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h > index 5eef8be3e59f..40c7e12cf09e 100644 > --- a/arch/sh/include/asm/page.h > +++ b/arch/sh/include/asm/page.h > @@ -205,4 +205,6 @@ typedef struct page *pgtable_t; > #define ARCH_SLAB_MINALIGN 8 > #endif > > +#define MAX_POSSIBLE_PHYSMEM_BITS 32 > + > #endif /* __ASM_SH_PAGE_H */ > diff --git a/arch/sparc/include/asm/page_32.h b/arch/sparc/include/asm/page_32.h > index b76d59edec8c..14e9ca4659d7 100644 > --- a/arch/sparc/include/asm/page_32.h > +++ b/arch/sparc/include/asm/page_32.h > @@ -139,4 +139,6 @@ extern unsigned long pfn_base; > #include > #include > > +#define MAX_POSSIBLE_PHYSMEM_BITS 32 > + > #endif /* _SPARC_PAGE_H */ > diff --git a/arch/sparc/include/asm/page_64.h b/arch/sparc/include/asm/page_64.h > index e80f2d5bf62f..6d6f3654ead1 100644 > --- a/arch/sparc/include/asm/page_64.h > +++ b/arch/sparc/include/asm/page_64.h > @@ -163,4 +163,6 @@ extern unsigned long PAGE_OFFSET; > > #include > > +#define MAX_POSSIBLE_PHYSMEM_BITS MAX_PHYS_ADDRESS_BITS > + > #endif /* _SPARC64_PAGE_H */ > diff --git a/arch/x86/include/asm/pgtable-2level_types.h b/arch/x86/include/asm/pgtable-2level_types.h > index 6deb6cd236e3..c2eae59e6505 100644 > --- a/arch/x86/include/asm/pgtable-2level_types.h > +++ b/arch/x86/include/asm/pgtable-2level_types.h > @@ -38,4 +38,6 @@ typedef union { > /* This covers all VMSPLIT_* and VMSPLIT_*_OPT variants */ > #define PGD_KERNEL_START (CONFIG_PAGE_OFFSET >> PGDIR_SHIFT) > > +#define MAX_POSSIBLE_PHYSMEM_BITS 32 > + > #endif /* _ASM_X86_PGTABLE_2LEVEL_DEFS_H */ > diff --git a/arch/x86/include/asm/pgtable-3level_types.h b/arch/x86/include/asm/pgtable-3level_types.h > index 33845d36897c..5fce514a49a0 100644 > --- a/arch/x86/include/asm/pgtable-3level_types.h > +++ b/arch/x86/include/asm/pgtable-3level_types.h > @@ -45,7 +45,8 @@ typedef union { > */ > #define PTRS_PER_PTE 512 > > -#define MAX_POSSIBLE_PHYSMEM_BITS 36 > #define PGD_KERNEL_START (CONFIG_PAGE_OFFSET >> PGDIR_SHIFT) > > +#define MAX_POSSIBLE_PHYSMEM_BITS 36 > + > #endif /* _ASM_X86_PGTABLE_3LEVEL_DEFS_H */ > diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h > index 84bd9bdc1987..d808cfde3d19 100644 > --- a/arch/x86/include/asm/pgtable_64_types.h > +++ b/arch/x86/include/asm/pgtable_64_types.h > @@ -64,8 +64,6 @@ extern unsigned int ptrs_per_p4d; > #define P4D_SIZE (_AC(1, UL) << P4D_SHIFT) > #define P4D_MASK (~(P4D_SIZE - 1)) > > -#define MAX_POSSIBLE_PHYSMEM_BITS 52 > - > #else /* CONFIG_X86_5LEVEL */ > > /* > @@ -154,4 +152,6 @@ extern unsigned int ptrs_per_p4d; > > #define PGD_KERNEL_START ((PAGE_SIZE / 2) / sizeof(pgd_t)) > > +#define MAX_POSSIBLE_PHYSMEM_BITS (pgtable_l5_enabled() ? 52 : 46) > + > #endif /* _ASM_X86_PGTABLE_64_DEFS_H */ > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c > index 0787d33b80d8..132c20b6fd4f 100644 > --- a/mm/zsmalloc.c > +++ b/mm/zsmalloc.c > @@ -80,23 +80,7 @@ > * as single (unsigned long) handle value. > * > * Note that object index starts from 0. > - * > - * This is made more complicated by various memory models and PAE. > - */ > - > -#ifndef MAX_POSSIBLE_PHYSMEM_BITS > -#ifdef MAX_PHYSMEM_BITS > -#define MAX_POSSIBLE_PHYSMEM_BITS MAX_PHYSMEM_BITS > -#else > -/* > - * If this definition of MAX_PHYSMEM_BITS is used, OBJ_INDEX_BITS will just > - * be PAGE_SHIFT > */ > -#define MAX_POSSIBLE_PHYSMEM_BITS BITS_PER_LONG > -#endif > -#endif > - > -#define _PFN_BITS (MAX_POSSIBLE_PHYSMEM_BITS - PAGE_SHIFT) > > /* > * Memory for allocating for handle keeps object position by > @@ -116,6 +100,25 @@ > */ > #define OBJ_ALLOCATED_TAG 1 > #define OBJ_TAG_BITS 1 > + > +/* > + * MAX_POSSIBLE_PHYSMEM_BITS should be defined by all archs using zsmalloc: > + * Trying to guess it from MAX_PHYSMEM_BITS, or considering it BITS_PER_LONG, > + * proved to be wrong by not considering PAE capabilities, or using SPARSEMEM > + * only headers, leading to bad object encoding due to object index overflow. > + */ > +#ifndef MAX_POSSIBLE_PHYSMEM_BITS > + #define MAX_POSSIBLE_PHYSMEM_BITS BITS_PER_LONG > + #error "MAX_POSSIBLE_PHYSMEM_BITS HAS to be defined by arch using zsmalloc"; > +#else > + #ifndef CONFIG_64BIT > + #if (MAX_POSSIBLE_PHYSMEM_BITS >= (BITS_PER_LONG + PAGE_SHIFT - OBJ_TAG_BITS)) > + #error "MAX_POSSIBLE_PHYSMEM_BITS is wrong for this arch"; > + #endif > + #endif > +#endif > + > +#define _PFN_BITS (MAX_POSSIBLE_PHYSMEM_BITS - PAGE_SHIFT) > #define OBJ_INDEX_BITS (BITS_PER_LONG - _PFN_BITS - OBJ_TAG_BITS) > #define OBJ_INDEX_MASK ((_AC(1, UL) << OBJ_INDEX_BITS) - 1) > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel