From: james.morse@arm.com (James Morse)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] arm64: print a fault message when attempting to write RO memory
Date: Tue, 04 Apr 2017 12:25:21 +0100 [thread overview]
Message-ID: <58E382A1.4070607@arm.com> (raw)
In-Reply-To: <20170404065803.4848-1-stephen.boyd@linaro.org>
Hi Stephen,
On 04/04/17 07:58, Stephen Boyd wrote:
> If a page is marked read only we should print out that fact,
> instead of printing out that there was a page fault. Right now we
> get a cryptic error message that something went wrong with an
> unhandled fault, but we don't evaluate the esr to figure out that
> it was a read/write permission fault.
>
> Instead of seeing:
>
> Unable to handle kernel paging request at virtual address ffff000008e460d8
> pgd = ffff800003504000
> [ffff000008e460d8] *pgd=0000000083473003, *pud=0000000083503003, *pmd=0000000000000000
> Internal error: Oops: 9600004f [#1] PREEMPT SMP
>
> we'll see:
>
> Unable to handle kernel write to read-only memory at virtual address ffff000008e760d8
> pgd = ffff80003d3de000
> [ffff000008e760d8] *pgd=0000000083472003, *pud=0000000083435003, *pmd=0000000000000000
> Internal error: Oops: 9600004f [#1] PREEMPT SMP
>
> We also fold the userspace address check into is_permission_fault()
> instead of at the current callsite so that the function can't be
> abused with software PAN and a kernel space address.
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 156169c6981b..c6560cb4ef50 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -321,7 +337,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
> mm_flags |= FAULT_FLAG_WRITE;
> }
>
> - if (addr < USER_DS && is_permission_fault(esr, regs)) {
> + if (is_permission_fault(esr, regs, addr)) {
> /* regs->orig_addr_limit may be 0 if we entered from EL0 */
> if (regs->orig_addr_limit == KERNEL_DS)
> die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
>
This change means the PAN checks claim permission faults on kernel addresses
too, we need to keep the addr check for these. (sorry, I missed this detail
first time round)
When I tried lkdtm's 'WRITE_RO' test it gave:
> [ 2114.718807] Internal error: Accessing user space memory outside uaccess.h
> routines: 9600004e [#1] PREEMPT SMP
With this hunk omitted I got the expected:
> [ 1476.243296] Unable to handle kernel write to read-only memory at virtual
> address ffff000008a11f10
I also gave this a spin on software-models with PAN and PAN+UAO, and TTBR0-PAN
on Juno.
With that hunk omitted:
Reviewed-by: James Morse <james.morse@arm.com>
Tested-by: James Morse <james.morse@arm.com>
Thanks,
James
WARNING: multiple messages have this Message-ID (diff)
From: James Morse <james.morse@arm.com>
To: Stephen Boyd <stephen.boyd@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will.deacon@arm.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Laura Abbott <labbott@redhat.com>,
Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH v3] arm64: print a fault message when attempting to write RO memory
Date: Tue, 04 Apr 2017 12:25:21 +0100 [thread overview]
Message-ID: <58E382A1.4070607@arm.com> (raw)
In-Reply-To: <20170404065803.4848-1-stephen.boyd@linaro.org>
Hi Stephen,
On 04/04/17 07:58, Stephen Boyd wrote:
> If a page is marked read only we should print out that fact,
> instead of printing out that there was a page fault. Right now we
> get a cryptic error message that something went wrong with an
> unhandled fault, but we don't evaluate the esr to figure out that
> it was a read/write permission fault.
>
> Instead of seeing:
>
> Unable to handle kernel paging request at virtual address ffff000008e460d8
> pgd = ffff800003504000
> [ffff000008e460d8] *pgd=0000000083473003, *pud=0000000083503003, *pmd=0000000000000000
> Internal error: Oops: 9600004f [#1] PREEMPT SMP
>
> we'll see:
>
> Unable to handle kernel write to read-only memory at virtual address ffff000008e760d8
> pgd = ffff80003d3de000
> [ffff000008e760d8] *pgd=0000000083472003, *pud=0000000083435003, *pmd=0000000000000000
> Internal error: Oops: 9600004f [#1] PREEMPT SMP
>
> We also fold the userspace address check into is_permission_fault()
> instead of at the current callsite so that the function can't be
> abused with software PAN and a kernel space address.
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 156169c6981b..c6560cb4ef50 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -321,7 +337,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
> mm_flags |= FAULT_FLAG_WRITE;
> }
>
> - if (addr < USER_DS && is_permission_fault(esr, regs)) {
> + if (is_permission_fault(esr, regs, addr)) {
> /* regs->orig_addr_limit may be 0 if we entered from EL0 */
> if (regs->orig_addr_limit == KERNEL_DS)
> die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
>
This change means the PAN checks claim permission faults on kernel addresses
too, we need to keep the addr check for these. (sorry, I missed this detail
first time round)
When I tried lkdtm's 'WRITE_RO' test it gave:
> [ 2114.718807] Internal error: Accessing user space memory outside uaccess.h
> routines: 9600004e [#1] PREEMPT SMP
With this hunk omitted I got the expected:
> [ 1476.243296] Unable to handle kernel write to read-only memory at virtual
> address ffff000008a11f10
I also gave this a spin on software-models with PAN and PAN+UAO, and TTBR0-PAN
on Juno.
With that hunk omitted:
Reviewed-by: James Morse <james.morse@arm.com>
Tested-by: James Morse <james.morse@arm.com>
Thanks,
James
next prev parent reply other threads:[~2017-04-04 11:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-04 6:58 [PATCH v3] arm64: print a fault message when attempting to write RO memory Stephen Boyd
2017-04-04 6:58 ` Stephen Boyd
2017-04-04 11:25 ` James Morse [this message]
2017-04-04 11:25 ` James Morse
2017-04-04 16:05 ` Laura Abbott
2017-04-04 16:05 ` Laura Abbott
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=58E382A1.4070607@arm.com \
--to=james.morse@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.