Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
On Tue, Feb 10, 2015 at 12:22:41PM +0400, Konstantin Khlebnikov wrote:
> I've found regression:
>
> [ 257.139907]
> [ 257.139909] [ BUG: lock held when returning to user space! ]
> [ 257.139912] 3.18.6-debug+ #161 Tainted: G U
> [ 257.139914]
> [ 257.139916] python/22843 is leaving the kernel with locks still held!
> [ 257.139918] 1 lock held by python/22843:
> [ 257.139920] #0: (&mm->mmap_sem){++}, at: []
> __do_page_fault+0x162/0x570
>
> upstream commit 7fb08eca45270d0ae86e1ad9d39c40b7a55d0190 must be backported
> too.
>
I guess the same regression can be found in the 3.16 kernel as it also
includes a backport of 33692f27597f ("vm: add VM_FAULT_SIGSEGV
handling support"). I'll queue 7fb08eca4527 ("x86: mm: move mmap_sem
unlock from mm_fault_error() to caller") as well.
Cheers,
--
Luís
> On Wed, Feb 4, 2015 at 2:13 AM, Greg Kroah-Hartman
> wrote:
> > 3.18-stable review patch. If anyone has any objections, please let me know.
> >
> > --
> >
> > From: Linus Torvalds
> >
> > commit 33692f27597fcab536d7cbbcc8f52905133e4aa7 upstream.
> >
> > The core VM already knows about VM_FAULT_SIGBUS, but cannot return a
> > "you should SIGSEGV" error, because the SIGSEGV case was generally
> > handled by the caller - usually the architecture fault handler.
> >
> > That results in lots of duplication - all the architecture fault
> > handlers end up doing very similar "look up vma, check permissions, do
> > retries etc" - but it generally works. However, there are cases where
> > the VM actually wants to SIGSEGV, and applications _expect_ SIGSEGV.
> >
> > In particular, when accessing the stack guard page, libsigsegv expects a
> > SIGSEGV. And it usually got one, because the stack growth is handled by
> > that duplicated architecture fault handler.
> >
> > However, when the generic VM layer started propagating the error return
> > from the stack expansion in commit fee7e49d4514 ("mm: propagate error
> > from stack expansion even for guard page"), that now exposed the
> > existing VM_FAULT_SIGBUS result to user space. And user space really
> > expected SIGSEGV, not SIGBUS.
> >
> > To fix that case, we need to add a VM_FAULT_SIGSEGV, and teach all those
> > duplicate architecture fault handlers about it. They all already have
> > the code to handle SIGSEGV, so it's about just tying that new return
> > value to the existing code, but it's all a bit annoying.
> >
> > This is the mindless minimal patch to do this. A more extensive patch
> > would be to try to gather up the mostly shared fault handling logic into
> > one generic helper routine, and long-term we really should do that
> > cleanup.
> >
> > Just from this patch, you can generally see that most architectures just
> > copied (directly or indirectly) the old x86 way of doing things, but in
> > the meantime that original x86 model has been improved to hold the VM
> > semaphore for shorter times etc and to handle VM_FAULT_RETRY and other
> > "newer" things, so it would be a good idea to bring all those
> > improvements to the generic case and teach other architectures about
> > them too.
> >
> > Reported-and-tested-by: Takashi Iwai
> > Tested-by: Jan Engelhardt
> > Acked-by: Heiko Carstens # "s390 still compiles
> > and boots"
> > Cc: linux-a...@vger.kernel.org
> > Signed-off-by: Linus Torvalds
> > Signed-off-by: Greg Kroah-Hartman
> >
> > ---
> > arch/alpha/mm/fault.c|2 ++
> > arch/arc/mm/fault.c |2 ++
> > arch/avr32/mm/fault.c|2 ++
> > arch/cris/mm/fault.c |2 ++
> > arch/frv/mm/fault.c |2 ++
> > arch/ia64/mm/fault.c |2 ++
> > arch/m32r/mm/fault.c |2 ++
> > arch/m68k/mm/fault.c |2 ++
> > arch/metag/mm/fault.c|2 ++
> > arch/microblaze/mm/fault.c |2 ++
> > arch/mips/mm/fault.c |2 ++
> > arch/mn10300/mm/fault.c |2 ++
> > arch/openrisc/mm/fault.c |2 ++
> > arch/parisc/mm/fault.c |2 ++
> > arch/powerpc/mm/copro_fault.c|2 +-
> > arch/powerpc/mm/fault.c |2 ++
> > arch/s390/mm/fault.c |6 ++
> > arch/score/mm/fault.c|2 ++
> > arch/sh/mm/fault.c |2 ++
> > arch/sparc/mm/fault_32.c |2 ++
> > arch/sparc/mm/fault_64.c |2 ++
> > arch/tile/mm/fault.c |2 ++
> > arch/um/kernel/trap.c|2 ++
> > arch/x86/mm/fault.c |2 ++
> > arch/xtensa/mm/fault.c |2 ++
> >
Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
On Wed, Feb 11, 2015 at 6:43 AM, Greg Kroah-Hartman
wrote:
> On Tue, Feb 10, 2015 at 12:22:41PM +0400, Konstantin Khlebnikov wrote:
>> I've found regression:
>>
>> [ 257.139907]
>> [ 257.139909] [ BUG: lock held when returning to user space! ]
>> [ 257.139912] 3.18.6-debug+ #161 Tainted: G U
>> [ 257.139914]
>> [ 257.139916] python/22843 is leaving the kernel with locks still held!
>> [ 257.139918] 1 lock held by python/22843:
>> [ 257.139920] #0: (&mm->mmap_sem){++}, at: []
>> __do_page_fault+0x162/0x570
>>
>> upstream commit 7fb08eca45270d0ae86e1ad9d39c40b7a55d0190 must be backported
>> too.
>
> Ah, nice, I missed that one. How did you test this?
I've catched hang on mmap_sem in some python self-test inside exherbo chroot.
With that patch test has finished successfully.
It seems the only way to tigger this is stack-overflow: for now VM_FAULT_SIGSEGV
is returned only if kernel cannot add guard page when stack expands.
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
On Tue, Feb 10, 2015 at 07:49:27PM -0800, Linus Torvalds wrote: > On Tue, Feb 10, 2015 at 7:43 PM, Greg Kroah-Hartman > wrote: > > > > Ah, nice, I missed that one. > > Ugh, to be fair, I missed it too. > > The alternative to backporting 7fb08eca4527 is to make the backport of > commit 33692f27597f use "bad_area()" instead of > "bad_area_nosemaphore()". 33692f27597f already showed up in 3.18.6, so I can't go back and change that version :( I'll just queue this one up, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
On Tue, Feb 10, 2015 at 7:43 PM, Greg Kroah-Hartman wrote: > > Ah, nice, I missed that one. Ugh, to be fair, I missed it too. The alternative to backporting 7fb08eca4527 is to make the backport of commit 33692f27597f use "bad_area()" instead of "bad_area_nosemaphore()". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
On Tue, Feb 10, 2015 at 12:22:41PM +0400, Konstantin Khlebnikov wrote:
> I've found regression:
>
> [ 257.139907]
> [ 257.139909] [ BUG: lock held when returning to user space! ]
> [ 257.139912] 3.18.6-debug+ #161 Tainted: G U
> [ 257.139914]
> [ 257.139916] python/22843 is leaving the kernel with locks still held!
> [ 257.139918] 1 lock held by python/22843:
> [ 257.139920] #0: (&mm->mmap_sem){++}, at: []
> __do_page_fault+0x162/0x570
>
> upstream commit 7fb08eca45270d0ae86e1ad9d39c40b7a55d0190 must be backported
> too.
Ah, nice, I missed that one. How did you test this?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
I've found regression:
[ 257.139907]
[ 257.139909] [ BUG: lock held when returning to user space! ]
[ 257.139912] 3.18.6-debug+ #161 Tainted: G U
[ 257.139914]
[ 257.139916] python/22843 is leaving the kernel with locks still held!
[ 257.139918] 1 lock held by python/22843:
[ 257.139920] #0: (&mm->mmap_sem){++}, at: []
__do_page_fault+0x162/0x570
upstream commit 7fb08eca45270d0ae86e1ad9d39c40b7a55d0190 must be backported too.
On Wed, Feb 4, 2015 at 2:13 AM, Greg Kroah-Hartman
wrote:
> 3.18-stable review patch. If anyone has any objections, please let me know.
>
> --
>
> From: Linus Torvalds
>
> commit 33692f27597fcab536d7cbbcc8f52905133e4aa7 upstream.
>
> The core VM already knows about VM_FAULT_SIGBUS, but cannot return a
> "you should SIGSEGV" error, because the SIGSEGV case was generally
> handled by the caller - usually the architecture fault handler.
>
> That results in lots of duplication - all the architecture fault
> handlers end up doing very similar "look up vma, check permissions, do
> retries etc" - but it generally works. However, there are cases where
> the VM actually wants to SIGSEGV, and applications _expect_ SIGSEGV.
>
> In particular, when accessing the stack guard page, libsigsegv expects a
> SIGSEGV. And it usually got one, because the stack growth is handled by
> that duplicated architecture fault handler.
>
> However, when the generic VM layer started propagating the error return
> from the stack expansion in commit fee7e49d4514 ("mm: propagate error
> from stack expansion even for guard page"), that now exposed the
> existing VM_FAULT_SIGBUS result to user space. And user space really
> expected SIGSEGV, not SIGBUS.
>
> To fix that case, we need to add a VM_FAULT_SIGSEGV, and teach all those
> duplicate architecture fault handlers about it. They all already have
> the code to handle SIGSEGV, so it's about just tying that new return
> value to the existing code, but it's all a bit annoying.
>
> This is the mindless minimal patch to do this. A more extensive patch
> would be to try to gather up the mostly shared fault handling logic into
> one generic helper routine, and long-term we really should do that
> cleanup.
>
> Just from this patch, you can generally see that most architectures just
> copied (directly or indirectly) the old x86 way of doing things, but in
> the meantime that original x86 model has been improved to hold the VM
> semaphore for shorter times etc and to handle VM_FAULT_RETRY and other
> "newer" things, so it would be a good idea to bring all those
> improvements to the generic case and teach other architectures about
> them too.
>
> Reported-and-tested-by: Takashi Iwai
> Tested-by: Jan Engelhardt
> Acked-by: Heiko Carstens # "s390 still compiles
> and boots"
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Linus Torvalds
> Signed-off-by: Greg Kroah-Hartman
>
> ---
> arch/alpha/mm/fault.c|2 ++
> arch/arc/mm/fault.c |2 ++
> arch/avr32/mm/fault.c|2 ++
> arch/cris/mm/fault.c |2 ++
> arch/frv/mm/fault.c |2 ++
> arch/ia64/mm/fault.c |2 ++
> arch/m32r/mm/fault.c |2 ++
> arch/m68k/mm/fault.c |2 ++
> arch/metag/mm/fault.c|2 ++
> arch/microblaze/mm/fault.c |2 ++
> arch/mips/mm/fault.c |2 ++
> arch/mn10300/mm/fault.c |2 ++
> arch/openrisc/mm/fault.c |2 ++
> arch/parisc/mm/fault.c |2 ++
> arch/powerpc/mm/copro_fault.c|2 +-
> arch/powerpc/mm/fault.c |2 ++
> arch/s390/mm/fault.c |6 ++
> arch/score/mm/fault.c|2 ++
> arch/sh/mm/fault.c |2 ++
> arch/sparc/mm/fault_32.c |2 ++
> arch/sparc/mm/fault_64.c |2 ++
> arch/tile/mm/fault.c |2 ++
> arch/um/kernel/trap.c|2 ++
> arch/x86/mm/fault.c |2 ++
> arch/xtensa/mm/fault.c |2 ++
> drivers/staging/lustre/lustre/llite/vvp_io.c |2 +-
> include/linux/mm.h |6 --
> mm/gup.c |4 ++--
> mm/ksm.c |2 +-
> 29 files changed, 61 insertions(+), 7 deletions(-)
>
> --- a/arch/alpha/mm/fault.c
> +++ b/arch/alpha/mm/fault.c
> @@ -156,6 +156,8 @@ retry:
> if (unlikely(fault & VM_FAULT_ERROR)) {
> if (fault & VM_FAULT_OOM)
> goto out_of_memory;
> +
[PATCH 3.18 04/57] vm: add VM_FAULT_SIGSEGV handling support
3.18-stable review patch. If anyone has any objections, please let me know.
--
From: Linus Torvalds
commit 33692f27597fcab536d7cbbcc8f52905133e4aa7 upstream.
The core VM already knows about VM_FAULT_SIGBUS, but cannot return a
"you should SIGSEGV" error, because the SIGSEGV case was generally
handled by the caller - usually the architecture fault handler.
That results in lots of duplication - all the architecture fault
handlers end up doing very similar "look up vma, check permissions, do
retries etc" - but it generally works. However, there are cases where
the VM actually wants to SIGSEGV, and applications _expect_ SIGSEGV.
In particular, when accessing the stack guard page, libsigsegv expects a
SIGSEGV. And it usually got one, because the stack growth is handled by
that duplicated architecture fault handler.
However, when the generic VM layer started propagating the error return
from the stack expansion in commit fee7e49d4514 ("mm: propagate error
from stack expansion even for guard page"), that now exposed the
existing VM_FAULT_SIGBUS result to user space. And user space really
expected SIGSEGV, not SIGBUS.
To fix that case, we need to add a VM_FAULT_SIGSEGV, and teach all those
duplicate architecture fault handlers about it. They all already have
the code to handle SIGSEGV, so it's about just tying that new return
value to the existing code, but it's all a bit annoying.
This is the mindless minimal patch to do this. A more extensive patch
would be to try to gather up the mostly shared fault handling logic into
one generic helper routine, and long-term we really should do that
cleanup.
Just from this patch, you can generally see that most architectures just
copied (directly or indirectly) the old x86 way of doing things, but in
the meantime that original x86 model has been improved to hold the VM
semaphore for shorter times etc and to handle VM_FAULT_RETRY and other
"newer" things, so it would be a good idea to bring all those
improvements to the generic case and teach other architectures about
them too.
Reported-and-tested-by: Takashi Iwai
Tested-by: Jan Engelhardt
Acked-by: Heiko Carstens # "s390 still compiles and
boots"
Cc: linux-a...@vger.kernel.org
Signed-off-by: Linus Torvalds
Signed-off-by: Greg Kroah-Hartman
---
arch/alpha/mm/fault.c|2 ++
arch/arc/mm/fault.c |2 ++
arch/avr32/mm/fault.c|2 ++
arch/cris/mm/fault.c |2 ++
arch/frv/mm/fault.c |2 ++
arch/ia64/mm/fault.c |2 ++
arch/m32r/mm/fault.c |2 ++
arch/m68k/mm/fault.c |2 ++
arch/metag/mm/fault.c|2 ++
arch/microblaze/mm/fault.c |2 ++
arch/mips/mm/fault.c |2 ++
arch/mn10300/mm/fault.c |2 ++
arch/openrisc/mm/fault.c |2 ++
arch/parisc/mm/fault.c |2 ++
arch/powerpc/mm/copro_fault.c|2 +-
arch/powerpc/mm/fault.c |2 ++
arch/s390/mm/fault.c |6 ++
arch/score/mm/fault.c|2 ++
arch/sh/mm/fault.c |2 ++
arch/sparc/mm/fault_32.c |2 ++
arch/sparc/mm/fault_64.c |2 ++
arch/tile/mm/fault.c |2 ++
arch/um/kernel/trap.c|2 ++
arch/x86/mm/fault.c |2 ++
arch/xtensa/mm/fault.c |2 ++
drivers/staging/lustre/lustre/llite/vvp_io.c |2 +-
include/linux/mm.h |6 --
mm/gup.c |4 ++--
mm/ksm.c |2 +-
29 files changed, 61 insertions(+), 7 deletions(-)
--- a/arch/alpha/mm/fault.c
+++ b/arch/alpha/mm/fault.c
@@ -156,6 +156,8 @@ retry:
if (unlikely(fault & VM_FAULT_ERROR)) {
if (fault & VM_FAULT_OOM)
goto out_of_memory;
+ else if (fault & VM_FAULT_SIGSEGV)
+ goto bad_area;
else if (fault & VM_FAULT_SIGBUS)
goto do_sigbus;
BUG();
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -161,6 +161,8 @@ good_area:
if (fault & VM_FAULT_OOM)
goto out_of_memory;
+ else if (fault & VM_FAULT_SIGSEV)
+ goto bad_area;
else if (fault & VM_FAULT_SIGBUS)
goto do_sigbus;
--- a/arch/avr32/mm/fault.c
+++ b/arch/avr32/mm/fault.c
@@ -142,6 +142,8 @@ good_area:
if (unlikely(fault & VM_FAULT_ERROR)) {
if (fault & VM_FAULT_OOM)
goto out_of_memory;
+ else if (fault & VM_FAULT_SIGSEGV)
+
