[{"id":678,"web_url":"http://patchwork.ozlabs.org/comment/678/","msgid":"","date":"2008-09-15T22:44:05","subject":"Re: [PATCH] powerpc: Avoid integer overflow in page_is_ram()","submitter":{"id":5,"url":"http://patchwork.ozlabs.org/api/people/5/","name":"Kumar Gala","email":"galak@kernel.crashing.org"},"content":"On Sep 15, 2008, at 3:43 PM, Roland Dreier wrote:\n\n> Commit 8b150478 (\"ppc: make phys_mem_access_prot() work with pfns\n> instead of addresses\") fixed page_is_ram() in arch/ppc to avoid \n> overflow\n> for addresses above 4G on 32-bit kernels. However arch/powerpc's\n> page_is_ram() is missing the same fix -- it computes a physical \n> address\n> by doing pfn << PAGE_SHIFT, which overflows if pfn corresponds to a \n> page\n> above 4G.\n>\n> In particular this causes pages above 4G to be mapped with the wrong\n> caching attribute; for example many ppc440-based SoCs have PCI space\n> above 4G, and mmap()ing MMIO space may end up with a mapping that has\n> caching enabled.\n>\n> Fix this by working with the pfn and avoiding the conversion to\n> physical address that causes the overflow. This patch compares the\n> pfn to max_pfn, which is a semantic change from the old code -- that\n> code compared the physical address to high_memory, which corresponds\n> to max_low_pfn. However, I think that was is another bug, since\n> highmem pages are still RAM.\n>\n> Reported-by: vb \n> Signed-off-by: Roland Dreier \n> Acked-by: Benjamin Herrenschmidt \n>\n> ---\n> Paul, didn't see this in your list... please add for 2.6.28.\n>\n> arch/powerpc/mm/mem.c | 5 ++---\n> 1 files changed, 2 insertions(+), 3 deletions(-)\n>\n> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c\n> index 1c93c25..98d7bf9 100644\n> --- a/arch/powerpc/mm/mem.c\n> +++ b/arch/powerpc/mm/mem.c\n> @@ -75,11 +75,10 @@ static inline pte_t *virt_to_kpte(unsigned long \n> vaddr)\n>\n> int page_is_ram(unsigned long pfn)\n> {\n> -\tunsigned long paddr = (pfn << PAGE_SHIFT);\n> -\n> #ifndef CONFIG_PPC64\t/* XXX for now */\n> -\treturn paddr < __pa(high_memory);\n> +\treturn pfn < max_pfn;\n> #else\n> +\tunsigned long paddr = (pfn << PAGE_SHIFT);\n\nseems like this could be a phys_addr_t\n\n>\n> \tint i;\n> \tfor (i=0; i < lmb.memory.cnt; i++) {\n> \t\tunsigned long base;\n\n- k","headers":{"Return-Path":"","X-Original-To":["patchwork@ozlabs.org","linuxppc-dev@ozlabs.org"],"Delivered-To":["patchwork@ozlabs.org","linuxppc-dev@ozlabs.org"],"Received":["from ozlabs.org (localhost [127.0.0.1])\n\tby ozlabs.org (Postfix) with ESMTP id CCE2BDE37D\n\tfor ; Tue, 16 Sep 2008 08:45:16 +1000 (EST)","from gate.crashing.org (gate.crashing.org [63.228.1.57])\n\t(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\n\t(Client did not present a certificate)\n\tby ozlabs.org (Postfix) with ESMTPS id D8E01DE04C\n\tfor ; Tue, 16 Sep 2008 08:44:30 +1000 (EST)","from [IPv6:::1] (localhost.localdomain [127.0.0.1])\n\tby gate.crashing.org (8.14.1/8.13.8) with ESMTP id m8FMi6Bp002647;\n\tMon, 15 Sep 2008 17:44:07 -0500"],"Message-Id":"","From":"Kumar Gala ","To":"Roland Dreier ","In-Reply-To":"","Mime-Version":"1.0 (Apple Message framework v926)","Subject":"Re: [PATCH] powerpc: Avoid integer overflow in page_is_ram()","Date":"Mon, 15 Sep 2008 17:44:05 -0500","References":"<18638.50702.962371.862911@cargo.ozlabs.ibm.com>\n\t","X-Mailer":"Apple Mail (2.926)","Cc":"linuxppc-dev@ozlabs.org, Paul Mackerras ","X-BeenThere":"linuxppc-dev@ozlabs.org","X-Mailman-Version":"2.1.11","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"; DelSp=\"yes\"","Sender":"linuxppc-dev-bounces+patchwork=ozlabs.org@ozlabs.org","Errors-To":"linuxppc-dev-bounces+patchwork=ozlabs.org@ozlabs.org"}},{"id":679,"web_url":"http://patchwork.ozlabs.org/comment/679/","msgid":"","date":"2008-09-15T22:51:35","subject":"Re: [PATCH] powerpc: Avoid integer overflow in page_is_ram()","submitter":{"id":64,"url":"http://patchwork.ozlabs.org/api/people/64/","name":"Roland Dreier","email":"rdreier@cisco.com"},"content":"> > #ifndef CONFIG_PPC64\t/* XXX for now */\n > > -\treturn paddr < __pa(high_memory);\n > > +\treturn pfn < max_pfn;\n > > #else\n > > +\tunsigned long paddr = (pfn << PAGE_SHIFT);\n > \n > seems like this could be a phys_addr_t\n\nYes, it could I guess, but that would be an unrelated change, and I'm\nnot sure there's much point given this is in 64-bit-only code.\n\n - R.","headers":{"Return-Path":"","X-Original-To":["patchwork@ozlabs.org","linuxppc-dev@ozlabs.org"],"Delivered-To":["patchwork@ozlabs.org","linuxppc-dev@ozlabs.org"],"Received":["from ozlabs.org (localhost [127.0.0.1])\n\tby ozlabs.org (Postfix) with ESMTP id F40ECDE259\n\tfor ; Tue, 16 Sep 2008 08:53:19 +1000 (EST)","from sj-iport-1.cisco.com (sj-iport-1.cisco.com [171.71.176.70])\n\t(using TLSv1 with cipher RC4-SHA (128/128 bits))\n\t(Client CN \"sj-iport-1.cisco.com\", Issuer \"Cisco SSCA\" (not verified))\n\tby ozlabs.org (Postfix) with ESMTPS id C81EADDEEF\n\tfor ; Tue, 16 Sep 2008 08:51:52 +1000 (EST)","from sj-dkim-2.cisco.com ([171.71.179.186])\n\tby sj-iport-1.cisco.com with ESMTP; 15 Sep 2008 22:51:38 +0000","from sj-core-5.cisco.com (sj-core-5.cisco.com [171.71.177.238])\n\tby sj-dkim-2.cisco.com (8.12.11/8.12.11) with ESMTP id m8FMpco7003604;\n\tMon, 15 Sep 2008 15:51:38 -0700","from xbh-sjc-231.amer.cisco.com (xbh-sjc-231.cisco.com\n\t[128.107.191.100])\n\tby sj-core-5.cisco.com (8.13.8/8.13.8) with ESMTP id m8FMpccw014297; \n\tMon, 15 Sep 2008 22:51:38 GMT","from xfe-sjc-212.amer.cisco.com ([171.70.151.187]) by\n\txbh-sjc-231.amer.cisco.com with Microsoft SMTPSVC(6.0.3790.1830); \n\tMon, 15 Sep 2008 15:51:36 -0700","from roland-conroe ([10.33.42.9]) by xfe-sjc-212.amer.cisco.com\n\twith\n\tMicrosoft SMTPSVC(6.0.3790.1830); Mon, 15 Sep 2008 15:51:35 -0700","by roland-conroe (Postfix, from userid 33217)\n\tid D9A211B64DA; Mon, 15 Sep 2008 15:51:35 -0700 (PDT)"],"From":"Roland Dreier ","To":"Kumar Gala ","Subject":"Re: [PATCH] powerpc: Avoid integer overflow in page_is_ram()","References":"<18638.50702.962371.862911@cargo.ozlabs.ibm.com>\n\t\n\t","X-Message-Flag":"Warning: May contain useful information","Date":"Mon, 15 Sep 2008 15:51:35 -0700","In-Reply-To":"\n\t(Kumar Gala's message of \"Mon, 15 Sep 2008 17:44:05 -0500\")","Message-ID":"","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux)","MIME-Version":"1.0","X-OriginalArrivalTime":"15 Sep 2008 22:51:36.0035 (UTC)\n\tFILETIME=[9B4CA330:01C91785]","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; l=374; t=1221519098;\n\tx=1222383098; c=relaxed/simple; s=sjdkim2002;\n\th=Content-Type:From:Subject:Content-Transfer-Encoding:MIME-Version;\n\td=cisco.com; i=rdreier@cisco.com;\n\tz=From:=20Roland=20Dreier=20\n\t|Subject:=20Re=3A=20[PATCH]=20powerpc=3A=20Avoid=20integer=\n\t20overflow=20in=20page_is_ram() |Sender:=20;\n\tbh=c0Q3WjdG315B2n/bm3A1yGrnqjTk9GE/gwzMdIKd2TY=;\n\tb=Zdmn1ld0paVCdMq7dkfmY33k9x88ri03I76px0nQmxHqag50348kVamQ6X\n\tVezVmjNpq+j2b7VUmCuAjdQnZlqcdPn7bwKdR7Dy1VqwXRraaZU6BP9RAOp8\n\tb0AU0pwuzd;","Authentication-Results":"sj-dkim-2; header.From=rdreier@cisco.com; dkim=pass (\n\tsig from cisco.com/sjdkim2002 verified; ); ","Cc":"linuxppc-dev@ozlabs.org, Paul Mackerras ","X-BeenThere":"linuxppc-dev@ozlabs.org","X-Mailman-Version":"2.1.11","Precedence":"list","List-Id":"Linux on PowerPC Developers Mail List ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Sender":"linuxppc-dev-bounces+patchwork=ozlabs.org@ozlabs.org","Errors-To":"linuxppc-dev-bounces+patchwork=ozlabs.org@ozlabs.org"}}]