From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: mm: check for upper PAGE_SHIFT bits in pfn_valid()
Date: Tue, 14 Aug 2018 11:40:42 +0100 [thread overview]
Message-ID: <20180814104041.GB28664@arm.com> (raw)
In-Reply-To: <20180813193013.236362-1-ghackmann@google.com>
Hi Greg,
On Mon, Aug 13, 2018 at 12:30:11PM -0700, Greg Hackmann wrote:
> ARM64's pfn_valid() shifts away the upper PAGE_SHIFT bits of the input
> before seeing if the PFN is valid. This leads to false positives when
> some of the upper bits are set, but the lower bits match a valid PFN.
>
> For example, the following userspace code looks up a bogus entry in
> /proc/kpageflags:
>
> int pagemap = open("/proc/self/pagemap", O_RDONLY);
> int pageflags = open("/proc/kpageflags", O_RDONLY);
> uint64_t pfn, val;
>
> lseek64(pagemap, [...], SEEK_SET);
> read(pagemap, &pfn, sizeof(pfn));
> if (pfn & (1UL << 63)) { /* valid PFN */
> pfn &= ((1UL << 55) - 1); /* clear flag bits */
> pfn |= (1UL << 55);
> lseek64(pageflags, pfn * sizeof(uint64_t), SEEK_SET);
> read(pageflags, &val, sizeof(val));
> }
>
> On ARM64 this causes the userspace process to crash with SIGSEGV rather
> than reading (1 << KPF_NOPAGE). kpageflags_read() treats the offset as
> valid, and stable_page_flags() will try to access an address between the
> user and kernel address ranges.
>
> Signed-off-by: Greg Hackmann <ghackmann@google.com>
> ---
> arch/arm64/mm/init.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Thanks, this looks like a sensible fix to me. Do you think it warrants a
CC stable?
Will
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9abf8a1e7b25..787e27964ab9 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -287,7 +287,11 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> int pfn_valid(unsigned long pfn)
> {
> - return memblock_is_map_memory(pfn << PAGE_SHIFT);
> + phys_addr_t addr = pfn << PAGE_SHIFT;
> +
> + if ((addr >> PAGE_SHIFT) != pfn)
> + return 0;
> + return memblock_is_map_memory(addr);
> }
> EXPORT_SYMBOL(pfn_valid);
> #endif
> --
> 2.18.0.597.ga71716f1ad-goog
>
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Greg Hackmann <ghackmann@android.com>
Cc: linux-arm-kernel@lists.infradead.org, kernel-team@android.com,
Greg Hackmann <ghackmann@google.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Andrew Morton <akpm@linux-foundation.org>,
Robin Murphy <robin.murphy@arm.com>,
Laura Abbott <labbott@redhat.com>,
Steve Capper <steve.capper@arm.com>,
Kristina Martsenko <kristina.martsenko@arm.com>,
Stefan Agner <stefan@agner.ch>,
CHANDAN VN <chandan.vn@samsung.com>,
Johannes Weiner <hannes@cmpxchg.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] arm64: mm: check for upper PAGE_SHIFT bits in pfn_valid()
Date: Tue, 14 Aug 2018 11:40:42 +0100 [thread overview]
Message-ID: <20180814104041.GB28664@arm.com> (raw)
In-Reply-To: <20180813193013.236362-1-ghackmann@google.com>
Hi Greg,
On Mon, Aug 13, 2018 at 12:30:11PM -0700, Greg Hackmann wrote:
> ARM64's pfn_valid() shifts away the upper PAGE_SHIFT bits of the input
> before seeing if the PFN is valid. This leads to false positives when
> some of the upper bits are set, but the lower bits match a valid PFN.
>
> For example, the following userspace code looks up a bogus entry in
> /proc/kpageflags:
>
> int pagemap = open("/proc/self/pagemap", O_RDONLY);
> int pageflags = open("/proc/kpageflags", O_RDONLY);
> uint64_t pfn, val;
>
> lseek64(pagemap, [...], SEEK_SET);
> read(pagemap, &pfn, sizeof(pfn));
> if (pfn & (1UL << 63)) { /* valid PFN */
> pfn &= ((1UL << 55) - 1); /* clear flag bits */
> pfn |= (1UL << 55);
> lseek64(pageflags, pfn * sizeof(uint64_t), SEEK_SET);
> read(pageflags, &val, sizeof(val));
> }
>
> On ARM64 this causes the userspace process to crash with SIGSEGV rather
> than reading (1 << KPF_NOPAGE). kpageflags_read() treats the offset as
> valid, and stable_page_flags() will try to access an address between the
> user and kernel address ranges.
>
> Signed-off-by: Greg Hackmann <ghackmann@google.com>
> ---
> arch/arm64/mm/init.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
Thanks, this looks like a sensible fix to me. Do you think it warrants a
CC stable?
Will
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9abf8a1e7b25..787e27964ab9 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -287,7 +287,11 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> int pfn_valid(unsigned long pfn)
> {
> - return memblock_is_map_memory(pfn << PAGE_SHIFT);
> + phys_addr_t addr = pfn << PAGE_SHIFT;
> +
> + if ((addr >> PAGE_SHIFT) != pfn)
> + return 0;
> + return memblock_is_map_memory(addr);
> }
> EXPORT_SYMBOL(pfn_valid);
> #endif
> --
> 2.18.0.597.ga71716f1ad-goog
>
next prev parent reply other threads:[~2018-08-14 10:40 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-13 19:30 [PATCH] arm64: mm: check for upper PAGE_SHIFT bits in pfn_valid() Greg Hackmann
2018-08-13 19:30 ` Greg Hackmann
2018-08-14 10:40 ` Will Deacon [this message]
2018-08-14 10:40 ` Will Deacon
2018-08-14 15:17 ` Greg Hackmann
2018-08-14 15:17 ` Greg Hackmann
2018-08-14 15:29 ` Will Deacon
2018-08-14 15:29 ` Will Deacon
2018-08-15 19:30 ` Greg Hackmann
2018-08-15 19:30 ` Greg Hackmann
-- strict thread matches above, loose matches on Subject: below --
2018-08-30 16:56 FAILED: patch "[PATCH] arm64: mm: check for upper PAGE_SHIFT bits in pfn_valid()" failed to apply to 4.4-stable tree gregkh
2018-08-30 18:25 ` [PATCH] arm64: mm: check for upper PAGE_SHIFT bits in pfn_valid() Greg Hackmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180814104041.GB28664@arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.