From: Liam Howlett <liam.howlett@oracle.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Julien Grall <julien.grall@arm.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>
Subject: Re: [PATCH 2/3] arm64: signal: sigreturn() and rt_sigreturn() sometime returns the wrong signals
Date: Fri, 30 Apr 2021 20:31:42 +0000 [thread overview]
Message-ID: <20210430203136.lnzqt7vuninh5v4t@revolver> (raw)
In-Reply-To: <m1y2cztuiw.fsf@fess.ebiederm.org>
* Eric W. Biederman <ebiederm@xmission.com> [210430 15:57]:
> Liam Howlett <liam.howlett@oracle.com> writes:
>
> > This is way out of scope for what I'm doing. I'm trying to fix a call
> > to the wrong mm API. I was trying to clean up any obvious errors in
> > calling functions which were exposed by fixing that error. If you want
> > this fixed differently, then please go ahead and tackle the problems you
> > see.
>
> I was asked by the arm maintainers to describe what the code should be
> doing here. I hope I have done that.
>
> What is very interesting is that the code in __do_page_fault does not
> use find_vma_intersection it uses find_vma. Which suggests that
> find_vma_intersection may not be the proper mm api.
>
> The logic is:
>
> From __do_page_fault:
> struct vm_area_struct *vma = find_vma(mm, addr);
>
> if (unlikely(!vma))
> return VM_FAULT_BADMAP;
>
> /*
> * Ok, we have a good vm_area for this memory access, so we can handle
> * it.
> */
> if (unlikely(vma->vm_start > addr)) {
> if (!(vma->vm_flags & VM_GROWSDOWN))
> return VM_FAULT_BADMAP;
> if (expand_stack(vma, addr))
> return VM_FAULT_BADMAP;
> }
>
> /*
> * Check that the permissions on the VMA allow for the fault which
> * occurred.
> */
> if (!(vma->vm_flags & vm_flags))
> return VM_FAULT_BADACCESS;
>
> From do_page_fault:
>
> arm64_force_sig_fault(SIGSEGV,
> fault == VM_FAULT_BADACCESS ? SEGV_ACCERR : SEGV_MAPERR,
> far, inf->name);
>
>
> Hmm. If the expand_stack step is skipped. Does is the logic equivalent
> to find_vma_intersection?
>
> static inline struct vm_area_struct *find_vma_intersection(
> struct mm_struct * mm,
> unsigned long start_addr,
> unsigned long end_addr)
> {
> struct vm_area_struct * vma = find_vma(mm,start_addr);
>
> if (vma && end_addr <= vma->vm_start)
> vma = NULL;
> return vma;
> }
>
> Yes. It does look that way. VM_FAULT_BADMAP is returned when a vma
> covering the specified address is not found. And VM_FAULT_BADACCESS is
> returned when there is a vma and there is a permission problem.
>
> There are also two SIGBUS cases that arm64_notify_segfault does not
> handle.
>
> So it appears changing arm64_notify_segfault to use
> find_vma_intersection instead of find_vma would be a correct but
> incomplete fix.
The reason VM_GROWSDOWN is checked here is to see if the stack should be
attempted to be expanded, which happens above. If VM_GROWSDOWN is true,
then wouldn't the do_page_fault() and __do_page_fault() calls already be
done and be handling this case?
>
> I don't see a point in changing sigerturn or rt_sigreturn.
Both functions call arm64_notify_segfault() on !access_ok() which I've
increased the potential for returning SEGV_MAPERR. It is also not
necessary to walk the VMAs when !access_ok(), so it seemed a decent
thing to do.
Thanks,
Liam
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-04-30 20:35 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-20 16:50 [PATCH 1/3] arm64: armv8_deprecated: Fix swp_handler() signal generation Liam Howlett
2021-04-20 16:50 ` [PATCH 2/3] arm64: signal: sigreturn() and rt_sigreturn() sometime returns the wrong signals Liam Howlett
2021-04-22 12:48 ` Will Deacon
2021-04-22 18:22 ` Eric W. Biederman
2021-04-22 19:24 ` Liam Howlett
2021-04-23 18:17 ` Eric W. Biederman
2021-04-23 20:03 ` Liam Howlett
2021-04-29 17:52 ` Eric W. Biederman
2021-04-30 18:48 ` Liam Howlett
2021-04-30 19:57 ` Eric W. Biederman
2021-04-30 20:31 ` Liam Howlett [this message]
2021-04-20 16:50 ` [PATCH 3/3] arch/arm64/kernel/traps: Use find_vma_intersection() in traps for setting si_code Liam Howlett
2021-04-22 12:53 ` Will Deacon
2021-04-22 18:56 ` Liam Howlett
[not found] ` <202104210515.VoF3YT1J-lkp@intel.com>
2021-04-21 15:47 ` [PATCH 1/3] arm64: armv8_deprecated: Fix swp_handler() signal generation Liam Howlett
2021-04-22 12:59 ` Will Deacon
2021-04-23 1:00 ` Liam Howlett
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=20210430203136.lnzqt7vuninh5v4t@revolver \
--to=liam.howlett@oracle.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=catalin.marinas@arm.com \
--cc=daniel@iogearbox.net \
--cc=ebiederm@xmission.com \
--cc=julien.grall@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=will@kernel.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 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).