From: Balbir Singh <bsingharora@gmail.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org,
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
Michael Neuling <michael.neuling@au1.ibm.com>,
Balbir Singh <bsingharora@gmail.com>
Subject: Re: [PATCH] powerpc/mm: Fix spurrious segfaults on radix with Autonuma
Date: Fri, 3 Feb 2017 16:28:22 +0530 [thread overview]
Message-ID: <20170203105822.GA23829@localhost.localdomain> (raw)
In-Reply-To: <1486102228.4850.52.camel@kernel.crashing.org>
On Fri, Feb 03, 2017 at 05:10:28PM +1100, Benjamin Herrenschmidt wrote:
> When autonuma marks a PTE inaccessible it clears all the protection
> bits but leave the PTE valid.
>
> With the Radix MMU, an attempt at executing from such a PTE will
> take a fault with bit 35 of SRR1 set "SRR1_ISI_N_OR_G".
>
> It is thus incorrect to treat all such faults as errors. We should
> pass them to handle_mm_fault() for autonuma to deal with. The case
> of pages that are really not executable is handled by the existing
> test for VM_EXEC further down.
>
> That leaves us with catching the kernel attempts at executing user
> pages. We can catch that earlier, even before we do find_vma.
>
> It is never valid on powerpc for the kernel to take an exec fault
> to begin with. So fold that test with the existing test for the
> kernel faulting on kernel addresses to bail out early.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Fixes: 1d18ad0 ("powerpc/mm: Detect instruction fetch denied and report")
> Fixes: 0ab5171 ("powerpc/mm: Fix no execute fault handling on pre-POWER5")
> ---
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 6fd30ac..62a50d6 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -253,8 +253,11 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
> if (unlikely(debugger_fault_handler(regs)))
> goto bail;
>
> - /* On a kernel SLB miss we can only check for a valid exception entry */
> - if (!user_mode(regs) && (address >= TASK_SIZE)) {
> + /*
> + * The kernel should never take an execute fault nor should it
> + * take a page fault to a kernel address.
> + */
> + if (!user_mode(regs) && (is_exec || (address >= TASK_SIZE))) {
> rc = SIGSEGV;
> goto bail;
> }
Aneesh did suggest a check for !user_mode(regs), but we did multiple combinations
of potential problems and decided the current check was OK. Not yet tested at my end,
I'll do that today.
Balbir Singh
next prev parent reply other threads:[~2017-02-03 10:58 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-03 6:10 [PATCH] powerpc/mm: Fix spurrious segfaults on radix with Autonuma Benjamin Herrenschmidt
2017-02-03 6:30 ` Aneesh Kumar K.V
2017-02-03 10:58 ` Balbir Singh [this message]
2017-02-06 10:45 ` Balbir Singh
2017-02-09 13:20 ` Michael Ellerman
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=20170203105822.GA23829@localhost.localdomain \
--to=bsingharora@gmail.com \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=benh@kernel.crashing.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=michael.neuling@au1.ibm.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).