[opensuse-factory] libsigsegv build fail with kernel 3.18.3
Dear Kernel hackers, We're having an interesting case that needs urgent diagnosis in Factory (as it's a ring-1 package, it blocks anything else from entering, as the staging areas all show the same issue). The issue: libsigsegv fails the test suite since the kernel has been updated to 3.18.3 (before we had 3.18.1). The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught). Are you aware of or can think of any change in the kernel that would cause this to happen? Or is the error the stackoverflow sample tries to catch, simply no longer meant to be caught by a sigsegv? One of the tests failing is the code to be found at http://git.savannah.gnu.org/cgit/libsigsegv.git/tree/tests/stackoverflow1.c Thanks a lot for your help! Dominique -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 2015-01-27 13:24, Dimstar / Dominique Leuenberger wrote:
Dear Kernel hackers,
We're having an interesting case that needs urgent diagnosis in Factory (as it's a ring-1 package, it blocks anything else from entering, as the staging areas all show the same issue).
The issue: libsigsegv fails the test suite since the kernel has been updated to 3.18.3 (before we had 3.18.1).
The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught).
Are you aware of or can think of any change in the kernel that would cause this to happen?
This is probably commit fee7e49d (mm: propagate error from stack expansion even for guard page).
Or is the error the stackoverflow sample tries to catch, simply no longer meant to be caught by a sigsegv?
I'm affraid it's either undefined behavior or the implementation is free to kill the process with whatever signal it wants :-/. Michal -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tuesday 2015-01-27 13:40, Michal Marek wrote:
On 2015-01-27 13:24, Dimstar / Dominique Leuenberger wrote:
The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught).
This is probably commit fee7e49d (mm: propagate error from stack expansion even for guard page).
Relatedly interesting: commit 320b2b8de12698082609ebbc1a17165727f4c893 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Thu Aug 12 17:54:33 2010 -0700 mm: keep a guard page below a grow-down stack segment This is a rather minimally invasive patch to solve the problem of the user stack growing into a memory mapped area below it. Whenever we fill the first page of the stack segment, expand the segment down by one page. Now, admittedly some odd application might _want_ the stack to grow down into the preceding memory mapping, and so we may at some point need to make this a process tunable (some people might also want to have more than a single page of guarding), but let's try the minimal approach first. Tested with trivial application that maps a single page just below the stack, and then starts recursing. Without this, we will get a SIGSEGV _after_ the stack has smashed the mapping. With this patch, we'll get a nice SIGBUS just as the stack touches the page just above the mapping. Whether it ought to be SIGBUS or SIGSEGV, is perhaps a matter of debate. WP has to say about it: """a process is trying to access memory that the CPU cannot physically address: an invalid address for the address bus, hence the name. [...] segmentation faults, which occur primarily due to memory access violations: problems in the logical address or permissions.""" The guard page, I would say, is an access violation protection, rather than something unadressable (like unaligned words on certain CPUs). -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Jan, On Tue, 2015-01-27 at 13:58 +0100, Jan Engelhardt wrote:
On Tuesday 2015-01-27 13:40, Michal Marek wrote:
On 2015-01-27 13:24, Dimstar / Dominique Leuenberger wrote:
The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught).
This is probably commit fee7e49d (mm: propagate error from stack expansion even for guard page).
Relatedly interesting:
commit 320b2b8de12698082609ebbc1a17165727f4c893 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Thu Aug 12 17:54:33 2010 -0700
mm: keep a guard page below a grow-down stack segment
This is a rather minimally invasive patch to solve the problem of the user stack growing into a memory mapped area below it. Whenever we fill the first page of the stack segment, expand the segment down by one page.
Now, admittedly some odd application might _want_ the stack to grow down into the preceding memory mapping, and so we may at some point need to make this a process tunable (some people might also want to have more than a single page of guarding), but let's try the minimal approach first.
Tested with trivial application that maps a single page just below the stack, and then starts recursing. Without this, we will get a SIGSEGV _after_ the stack has smashed the mapping. With this patch, we'll get a nice SIGBUS just as the stack touches the page just above the mapping.
Whether it ought to be SIGBUS or SIGSEGV, is perhaps a matter of debate. WP has to say about it:
That one sounds actually very much like the kernel changes this behavior intentionally and 100% conscious.
"""a process is trying to access memory that the CPU cannot physically address: an invalid address for the address bus, hence the name. [...] segmentation faults, which occur primarily due to memory access violations: problems in the logical address or permissions."""
The guard page, I would say, is an access violation protection, rather than something unadressable (like unaligned words on certain CPUs).
Indeed, semantics. Somebody should ask Linus about it :) IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that. osc whatdependson gives me this short list: libsigsegv : cedilla clisp emacs-auctex maxima texlive wxMaxima xindy yodl Thoughts anybody? -- Dimstar / Dominique Leuenberger <dimstar@opensuse.org> -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Dimstar / Dominique Leuenberger <dimstar@opensuse.org> writes:
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
If libsigsegv can no longer detect stack overflow it becomes pointless. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, 2015-01-27 at 14:20 +0100, Andreas Schwab wrote:
Dimstar / Dominique Leuenberger <dimstar@opensuse.org> writes:
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
If libsigsegv can no longer detect stack overflow it becomes pointless.
I looked a bit deeper in its code... seems there are similar such things already done for Mac-OS and other platforms (hurd, Macos, ...) I patched signals.h to have it handle SIGBUS the same way as it does for those platforms now... first tests build already again. -- Dimstar / Dominique Leuenberger <dimstar@opensuse.org> -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tuesday 2015-01-27 14:20, Andreas Schwab wrote:
Dimstar / Dominique Leuenberger <dimstar@opensuse.org> writes:
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
If libsigsegv can no longer detect stack overflow it becomes pointless.
Well no, it can still react to writes to unmapped regions. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Jan Engelhardt <jengelh@inai.de> writes:
On Tuesday 2015-01-27 14:20, Andreas Schwab wrote:
Dimstar / Dominique Leuenberger <dimstar@opensuse.org> writes:
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
If libsigsegv can no longer detect stack overflow it becomes pointless.
Well no, it can still react to writes to unmapped regions.
Stack overflows are its main use case. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, 27 Jan 2015 14:08, Dimstar / Dominique Leuenberger <dimstar@...> wrote:
On Tue, 2015-01-27 at 13:58 +0100, Jan Engelhardt wrote:
On Tuesday 2015-01-27 13:40, Michal Marek wrote:
On 2015-01-27 13:24, Dimstar / Dominique Leuenberger wrote:
The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught).
This is probably commit fee7e49d (mm: propagate error from stack expansion even for guard page).
Relatedly interesting:
commit 320b2b8de12698082609ebbc1a17165727f4c893 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Thu Aug 12 17:54:33 2010 -0700
mm: keep a guard page below a grow-down stack segment
This is a rather minimally invasive patch to solve the problem of the user stack growing into a memory mapped area below it. Whenever we fill the first page of the stack segment, expand the segment down by one page.
Now, admittedly some odd application might _want_ the stack to grow down into the preceding memory mapping, and so we may at some point need to make this a process tunable (some people might also want to have more than a single page of guarding), but let's try the minimal approach first.
Tested with trivial application that maps a single page just below the stack, and then starts recursing. Without this, we will get a SIGSEGV _after_ the stack has smashed the mapping. With this patch, we'll get a nice SIGBUS just as the stack touches the page just above the mapping.
Whether it ought to be SIGBUS or SIGSEGV, is perhaps a matter of debate. WP has to say about it:
That one sounds actually very much like the kernel changes this behavior intentionally and 100% conscious.
"""a process is trying to access memory that the CPU cannot physically address: an invalid address for the address bus, hence the name. [...] segmentation faults, which occur primarily due to memory access violations: problems in the logical address or permissions."""
The guard page, I would say, is an access violation protection, rather than something unadressable (like unaligned words on certain CPUs).
Indeed, semantics. Somebody should ask Linus about it :)
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
osc whatdependson gives me this short list: libsigsegv : cedilla clisp emacs-auctex maxima texlive wxMaxima xindy yodl
Thoughts anybody?
Due to the patch mentioned above the stack breaking the segment is caught BEFORE it happens, thus SIGBUS, so no more SIGSEGV that way. Either libsigsegv is modified to handle SIGBUS for Kernel >=3.18.3 the same as SIGSEGV, or raise the question upstream why those projects depend on libsigsegv at all. For me the pure 'need' to use libsigsegv in a project beyond testing is cause for rised hackles and premotions of BAD coding. Either way, here is upstream in question. - libsigsegv to handle SIGBUS for kernel >= 3.18.3 - the dependend projects to eliminate need for libsigsegv at all. For us here? - SIGBUS hack is the fasted way to go forward ATM. - Yamaban. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Yamaban <foerster@lisas.de> writes:
> For me the pure 'need' to use libsigsegv in a project beyond testing
> is cause for rised hackles and premotions of BAD coding.
The point of libsigsegv is to handle stack overflows gracefully, instead
of just crashing.
> Either way, here is upstream in question.
> - libsigsegv to handle SIGBUS for kernel >= 3.18.3
> - the dependend projects to eliminate need for
> libsigsegv at all.
- the kernel to eliminate a regression (tell Linus and he will agree
violently)
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
--
To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org
To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Linus, the commit [320b2b8de126: mm: keep a guard page below a grow-down stack segment] seems resulting in a regression with libsigsegv. Looking at the change, it's pretty obvious why it breaks; libsigsegv expects SIGSEGV explicitly for the scenario the patch touches. (FWIW, a failing test code to be found at: http://git.savannah.gnu.org/cgit/libsigsegv.git/tree/tests/stackoverflow1.c ) This is a case giving us a dilemma. Of course, we can "fix" libsigsegv to handle SIGBUS gracefully. OTOH, this can be seen as a user-space regression we'd like to avoid usually. What do you think? thanks, Takashi At Tue, 27 Jan 2015 14:08:04 +0100, Dimstar / Dominique Leuenberger wrote:
Jan,
On Tue, 2015-01-27 at 13:58 +0100, Jan Engelhardt wrote:
On Tuesday 2015-01-27 13:40, Michal Marek wrote:
On 2015-01-27 13:24, Dimstar / Dominique Leuenberger wrote:
The test suite is intentionally 'badly written' and is supposed to cause sigsegv for various cases. The current tests failing are around stack overflows. Up to kernel 3.18.1, those reported SIGSEGV back, but it seems nowadays we get a SIGBUS (which is not in scope of a library called libSIGSEGV to be caught).
This is probably commit fee7e49d (mm: propagate error from stack expansion even for guard page).
Relatedly interesting:
commit 320b2b8de12698082609ebbc1a17165727f4c893 Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Thu Aug 12 17:54:33 2010 -0700
mm: keep a guard page below a grow-down stack segment
This is a rather minimally invasive patch to solve the problem of the user stack growing into a memory mapped area below it. Whenever we fill the first page of the stack segment, expand the segment down by one page.
Now, admittedly some odd application might _want_ the stack to grow down into the preceding memory mapping, and so we may at some point need to make this a process tunable (some people might also want to have more than a single page of guarding), but let's try the minimal approach first.
Tested with trivial application that maps a single page just below the stack, and then starts recursing. Without this, we will get a SIGSEGV _after_ the stack has smashed the mapping. With this patch, we'll get a nice SIGBUS just as the stack touches the page just above the mapping.
Whether it ought to be SIGBUS or SIGSEGV, is perhaps a matter of debate. WP has to say about it:
That one sounds actually very much like the kernel changes this behavior intentionally and 100% conscious.
"""a process is trying to access memory that the CPU cannot physically address: an invalid address for the address bus, hence the name. [...] segmentation faults, which occur primarily due to memory access violations: problems in the logical address or permissions."""
The guard page, I would say, is an access violation protection, rather than something unadressable (like unaligned words on certain CPUs).
Indeed, semantics. Somebody should ask Linus about it :)
IF this is to remain SIGBUS, then I don't think it's (anymore) in scope of libsigsegv to catch it and offer a handler for it, in which case removing the test would be the logical step; but I don't know what all else we might break with that.
osc whatdependson gives me this short list: libsigsegv : cedilla clisp emacs-auctex maxima texlive wxMaxima xindy yodl
Thoughts anybody?
-- Dimstar / Dominique Leuenberger <dimstar@opensuse.org>
-- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
-- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 11:32 AM, Takashi Iwai <tiwai@suse.de> wrote:
the commit [320b2b8de126: mm: keep a guard page below a grow-down stack segment] seems resulting in a regression with libsigsegv.
This is a case giving us a dilemma. Of course, we can "fix" libsigsegv to handle SIGBUS gracefully. OTOH, this can be seen as a user-space regression we'd like to avoid usually.
Yeah, no, that's obviously a kernel regression and needs to be fixed, even if no sane program should care about SIGSEGV vs SIGBUS. But "sane program" is irrelevant for the regression rule - somebody noticed. The only reason it returns SIGBUS is that we don't have an equivalent VM_FAULT_SIGSEGV error return. Which is something of an oversight, but adding it is a pain too, simply because the callers are mainly the architecture page fault handlers, so the VM_FAULT_SIGSEGV handling has to be added to all of them. Very annoying. The patch would look something like the attached - TOTALLY UNTESTED. Does this fix things for you? Linus
At Tue, 27 Jan 2015 12:36:38 -0800, Linus Torvalds wrote:
On Tue, Jan 27, 2015 at 11:32 AM, Takashi Iwai <tiwai@suse.de> wrote:
the commit [320b2b8de126: mm: keep a guard page below a grow-down stack segment] seems resulting in a regression with libsigsegv.
This is a case giving us a dilemma. Of course, we can "fix" libsigsegv to handle SIGBUS gracefully. OTOH, this can be seen as a user-space regression we'd like to avoid usually.
Yeah, no, that's obviously a kernel regression and needs to be fixed, even if no sane program should care about SIGSEGV vs SIGBUS. But "sane program" is irrelevant for the regression rule - somebody noticed.
Oh yeah, I meant kernel regression, too, sorry for confusion. (Maybe I chose the wrong word unconsciously as a kernel hacker ;)
The only reason it returns SIGBUS is that we don't have an equivalent VM_FAULT_SIGSEGV error return. Which is something of an oversight, but adding it is a pain too, simply because the callers are mainly the architecture page fault handlers, so the VM_FAULT_SIGSEGV handling has to be added to all of them.
Very annoying. The patch would look something like the attached - TOTALLY UNTESTED.
Does this fix things for you?
Unfortunately it doesn't seem to work. I compiled and test quickly, but two of five test still gives SIGBUS (stackoverflow1 and stackoverflow2). FYI, I'm testing libsigsegv-2.10, that can be downloaded: http://www.gnu.org/software/libsigsegv/#Download and just make, make check. It should be easily testable on your machine, too. thanks, Takashi
Linus 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/nios2/mm/fault.c | 2 ++ arch/openrisc/mm/fault.c | 2 ++ arch/parisc/mm/fault.c | 2 ++ arch/powerpc/mm/copro_fault.c | 3 +++ 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 ++ include/linux/mm.h | 6 ++++-- 27 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/alpha/mm/fault.c b/arch/alpha/mm/fault.c index 98838a05ba6d..9d0ac091a52a 100644 --- 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(); diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index 6f7e3a68803a..0f8df3b5b1b3 100644 --- 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;
diff --git a/arch/avr32/mm/fault.c b/arch/avr32/mm/fault.c index 0eca93327195..d223a8b57c1e 100644 --- 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) + goto bad_area; else if (fault & VM_FAULT_SIGBUS) goto do_sigbus; BUG(); diff --git a/arch/cris/mm/fault.c b/arch/cris/mm/fault.c index 1790f22e71a2..2686a7aa8ec8 100644 --- a/arch/cris/mm/fault.c +++ b/arch/cris/mm/fault.c @@ -176,6 +176,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(); diff --git a/arch/frv/mm/fault.c b/arch/frv/mm/fault.c index 9a66372fc7c7..ec4917ddf678 100644 --- a/arch/frv/mm/fault.c +++ b/arch/frv/mm/fault.c @@ -168,6 +168,8 @@ asmlinkage void do_page_fault(int datammu, unsigned long esr0, unsigned long ear 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(); diff --git a/arch/ia64/mm/fault.c b/arch/ia64/mm/fault.c index 7225dad87094..ba5ba7accd0d 100644 --- a/arch/ia64/mm/fault.c +++ b/arch/ia64/mm/fault.c @@ -172,6 +172,8 @@ retry: */ if (fault & VM_FAULT_OOM) { goto out_of_memory; + } else if (fault & VM_FAULT_SIGSEGV) { + goto bad_area; } else if (fault & VM_FAULT_SIGBUS) { signal = SIGBUS; goto bad_area; diff --git a/arch/m32r/mm/fault.c b/arch/m32r/mm/fault.c index e9c6a8014bd6..e3d4d4890104 100644 --- a/arch/m32r/mm/fault.c +++ b/arch/m32r/mm/fault.c @@ -200,6 +200,8 @@ good_area: 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(); diff --git a/arch/m68k/mm/fault.c b/arch/m68k/mm/fault.c index 2bd7487440c4..b2f04aee46ec 100644 --- a/arch/m68k/mm/fault.c +++ b/arch/m68k/mm/fault.c @@ -145,6 +145,8 @@ good_area: if (unlikely(fault & VM_FAULT_ERROR)) { if (fault & VM_FAULT_OOM) goto out_of_memory; + else if (fault & VM_FAULT_SIGSEGV) + goto map_err; else if (fault & VM_FAULT_SIGBUS) goto bus_err; BUG(); diff --git a/arch/metag/mm/fault.c b/arch/metag/mm/fault.c index 332680e5ebf2..2de5dc695a87 100644 --- a/arch/metag/mm/fault.c +++ b/arch/metag/mm/fault.c @@ -141,6 +141,8 @@ good_area: 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(); diff --git a/arch/microblaze/mm/fault.c b/arch/microblaze/mm/fault.c index fa4cf52aa7a6..d46a5ebb7570 100644 --- a/arch/microblaze/mm/fault.c +++ b/arch/microblaze/mm/fault.c @@ -224,6 +224,8 @@ good_area: 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(); diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c index becc42bb1849..70ab5d664332 100644 --- a/arch/mips/mm/fault.c +++ b/arch/mips/mm/fault.c @@ -158,6 +158,8 @@ good_area: 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(); diff --git a/arch/mn10300/mm/fault.c b/arch/mn10300/mm/fault.c index 3516cbdf1ee9..0c2cc5d39c8e 100644 --- a/arch/mn10300/mm/fault.c +++ b/arch/mn10300/mm/fault.c @@ -262,6 +262,8 @@ good_area: 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(); diff --git a/arch/nios2/mm/fault.c b/arch/nios2/mm/fault.c index 15a0bb5fc06d..34429d5a0ccd 100644 --- a/arch/nios2/mm/fault.c +++ b/arch/nios2/mm/fault.c @@ -135,6 +135,8 @@ survive: 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(); diff --git a/arch/openrisc/mm/fault.c b/arch/openrisc/mm/fault.c index 0703acf7d327..0a0c5fa1a64f 100644 --- a/arch/openrisc/mm/fault.c +++ b/arch/openrisc/mm/fault.c @@ -171,6 +171,8 @@ good_area: if (unlikely(fault & VM_FAULT_ERROR)) { if (fault & VM_FAULT_OOM) goto out_of_memory; + else if (faulr & VM_FAULT_SIGSEGV) + goto bad_area; else if (fault & VM_FAULT_SIGBUS) goto do_sigbus; BUG(); diff --git a/arch/parisc/mm/fault.c b/arch/parisc/mm/fault.c index 3ca9c1131cfe..e5120e653240 100644 --- a/arch/parisc/mm/fault.c +++ b/arch/parisc/mm/fault.c @@ -256,6 +256,8 @@ good_area: */ 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 bad_area; BUG(); diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c index 5a236f082c78..094b7d445023 100644 --- a/arch/powerpc/mm/copro_fault.c +++ b/arch/powerpc/mm/copro_fault.c @@ -79,6 +79,9 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea, } else if (*flt & VM_FAULT_SIGBUS) { ret = -EFAULT; goto out_unlock; + } else if (*flt & VM_FAULT_SIGBUS) { + ret = -EFAULT; + goto out_unlock; } BUG(); } diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index eb79907f34fa..6154b0a2b063 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -437,6 +437,8 @@ good_area: */ fault = handle_mm_fault(mm, vma, address, flags); if (unlikely(fault & (VM_FAULT_RETRY|VM_FAULT_ERROR))) { + if (fault & VM_FAULT_SIGSEGV) + goto bad_area; rc = mm_fault_error(regs, address, fault); if (rc >= MM_FAULT_RETURN) goto bail; diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index 811937bb90be..9065d5aa3932 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -374,6 +374,12 @@ static noinline void do_fault_error(struct pt_regs *regs, int fault) do_no_context(regs); else pagefault_out_of_memory(); + } else if (fault & VM_FAULT_SIGSEGV) { + /* Kernel mode? Handle exceptions or die */ + if (!user_mode(regs)) + do_no_context(regs); + else + do_sigsegv(regs, SEGV_MAPERR); } else if (fault & VM_FAULT_SIGBUS) { /* Kernel mode? Handle exceptions or die */ if (!user_mode(regs)) diff --git a/arch/score/mm/fault.c b/arch/score/mm/fault.c index 52238983527d..6860beb2a280 100644 --- a/arch/score/mm/fault.c +++ b/arch/score/mm/fault.c @@ -114,6 +114,8 @@ good_area: 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(); diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c index 541dc6101508..a58fec9b55e0 100644 --- a/arch/sh/mm/fault.c +++ b/arch/sh/mm/fault.c @@ -353,6 +353,8 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code, } else { if (fault & VM_FAULT_SIGBUS) do_sigbus(regs, error_code, address); + else if (fault & VM_FAULT_SIGSEGV) + bad_area(regs, error_code, address); else BUG(); } diff --git a/arch/sparc/mm/fault_32.c b/arch/sparc/mm/fault_32.c index 908e8c17c902..70d817154fe8 100644 --- a/arch/sparc/mm/fault_32.c +++ b/arch/sparc/mm/fault_32.c @@ -249,6 +249,8 @@ good_area: 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(); diff --git a/arch/sparc/mm/fault_64.c b/arch/sparc/mm/fault_64.c index 18fcd7167095..479823249429 100644 --- a/arch/sparc/mm/fault_64.c +++ b/arch/sparc/mm/fault_64.c @@ -446,6 +446,8 @@ good_area: 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(); diff --git a/arch/tile/mm/fault.c b/arch/tile/mm/fault.c index 565e25a98334..0f61a73534e6 100644 --- a/arch/tile/mm/fault.c +++ b/arch/tile/mm/fault.c @@ -442,6 +442,8 @@ good_area: 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(); diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c index 5678c3571e7c..209617302df8 100644 --- a/arch/um/kernel/trap.c +++ b/arch/um/kernel/trap.c @@ -80,6 +80,8 @@ good_area: if (unlikely(fault & VM_FAULT_ERROR)) { if (fault & VM_FAULT_OOM) { goto out_of_memory; + } else if (fault & VM_FAULT_SIGSEGV) { + goto out; } else if (fault & VM_FAULT_SIGBUS) { err = -EACCES; goto out; diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 38dcec403b46..e3ff27a5b634 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -898,6 +898,8 @@ mm_fault_error(struct pt_regs *regs, unsigned long error_code, if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON| VM_FAULT_HWPOISON_LARGE)) do_sigbus(regs, error_code, address, fault); + else if (fault & VM_FAULT_SIGSEGV) + bad_area_nosemaphore(regs, error_code, address); else BUG(); } diff --git a/arch/xtensa/mm/fault.c b/arch/xtensa/mm/fault.c index b57c4f91f487..9e3571a6535c 100644 --- a/arch/xtensa/mm/fault.c +++ b/arch/xtensa/mm/fault.c @@ -117,6 +117,8 @@ good_area: 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(); diff --git a/include/linux/mm.h b/include/linux/mm.h index 80fc92a49649..dd5ea3016fc4 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1070,6 +1070,7 @@ static inline int page_mapped(struct page *page) #define VM_FAULT_WRITE 0x0008 /* Special case for get_user_pages */ #define VM_FAULT_HWPOISON 0x0010 /* Hit poisoned small page */ #define VM_FAULT_HWPOISON_LARGE 0x0020 /* Hit poisoned large page. Index encoded in upper bits */ +#define VM_FAULT_SIGSEGV 0x0040
#define VM_FAULT_NOPAGE 0x0100 /* ->fault installed the pte, not return page */ #define VM_FAULT_LOCKED 0x0200 /* ->fault locked the returned page */ @@ -1078,8 +1079,9 @@ static inline int page_mapped(struct page *page)
#define VM_FAULT_HWPOISON_LARGE_MASK 0xf000 /* encodes hpage index for large hwpoison */
-#define VM_FAULT_ERROR (VM_FAULT_OOM | VM_FAULT_SIGBUS | VM_FAULT_HWPOISON | \ - VM_FAULT_FALLBACK | VM_FAULT_HWPOISON_LARGE) +#define VM_FAULT_ERROR (VM_FAULT_OOM | VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | \ + VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE | \ + VM_FAULT_FALLBACK)
/* Encode hstate index for a hwpoisoned large page */ #define VM_FAULT_SET_HINDEX(x) ((x) << 12)
-- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 12:56 PM, Takashi Iwai <tiwai@suse.de> wrote:
Unfortunately it doesn't seem to work. I compiled and test quickly, but two of five test still gives SIGBUS (stackoverflow1 and stackoverflow2).
Oh, I forgot to make it clear that the patch was _purely_ the "infrastructure" patch to add support for VM_FAULT_SIGSEGV. To actually make the stack guard page access failure give SIGSEGV, you'd need to also then change that if (check_stack_guard_page(vma, address) < 0) return VM_FAULT_SIGBUS; in mm/memory.c to actually use the new "VM_FAULT_SIGSEGV" thing. Otherwise it will obviously continue to send SIGSBUS. Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
At Tue, 27 Jan 2015 13:00:36 -0800, Linus Torvalds wrote:
On Tue, Jan 27, 2015 at 12:56 PM, Takashi Iwai <tiwai@suse.de> wrote:
Unfortunately it doesn't seem to work. I compiled and test quickly, but two of five test still gives SIGBUS (stackoverflow1 and stackoverflow2).
Oh, I forgot to make it clear that the patch was _purely_ the "infrastructure" patch to add support for VM_FAULT_SIGSEGV.
To actually make the stack guard page access failure give SIGSEGV, you'd need to also then change that
if (check_stack_guard_page(vma, address) < 0) return VM_FAULT_SIGBUS;
in mm/memory.c to actually use the new "VM_FAULT_SIGSEGV" thing. Otherwise it will obviously continue to send SIGSBUS.
OK, got it. Now the result is positive. libsigsegv "make check" passed all tests. (tested with your second patch + this change) Do you have a test case that should still generate SIGBUS, too? Thanks! Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 1:41 PM, Takashi Iwai <tiwai@suse.de> wrote:
Do you have a test case that should still generate SIGBUS, too?
Nope, but it should be easy to generate: doing a shared mmap() that is larger than the file you're mmap'ing, and then trying to access a page past the end of the file. That's the traditional way to get SIGBUS. Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
At Tue, 27 Jan 2015 13:55:08 -0800, Linus Torvalds wrote:
On Tue, Jan 27, 2015 at 1:41 PM, Takashi Iwai <tiwai@suse.de> wrote:
Do you have a test case that should still generate SIGBUS, too?
Nope, but it should be easy to generate: doing a shared mmap() that is larger than the file you're mmap'ing, and then trying to access a page past the end of the file. That's the traditional way to get SIGBUS.
Yeah, thanks. I just misunderstood the patch as if there were still cases with stack limit violation triggering SIGBUS. Forget my question above. I need to go to bed... In anyway, feel free to add my tested-by tag Tested-by: Takashi Iwai <tiwai@suse.de> I'll merge the patch into SUSE kernels for wider testing once when the patch is finalized. Thanks! Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 2:07 PM, Takashi Iwai <tiwai@suse.de> wrote:
I'll merge the patch into SUSE kernels for wider testing once when the patch is finalized.
I'd hope from some noise from the arch maintainers, although judging by past experience that is probably a rather naive hope. I think most arch people live in their own private mailing lists, and linux-arch isn't actually that widely read. So more likely is that I'll end up committing it unchanged in a few days and then people will complain if it breaks their architecture. And if you don't see the commit happening, feel free to remind me (but I'll keep the patch dirtying my tree so I hopefully shouldn't forget it). Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
[ Adding 'linux-arch' to the recipients, since this touches pretty much all architectures ] Background for arch people: it seems that a few applications really care about the difference between SIGSEGV and SIGBUS, but the generic VM layer currently has no way to say "this access should generate a SIGSEGV". We have that VM_FAULT_SIGBUS, but no equivalent VM_FAULT_SIGSEGV. So when the stack limit fix went in, I used VM_FAULT_SIGBUS, and a couple of apps noticed that the stack rlimit violation changed from SIGSEGV to SIGBUS as a result. It's actually sad that this whole error handling is duplicated all over every architecture, but oh well. This is a completely mindless patch to add VM_FAULT_SIGSEGV. Some architectures aren't affected, for the simple reason that they already ended up returning SIGSEGV for non-SIGBUS errors. Most other architectures had a BUG_ON() for the unrecognized case, and just need a trivial "if (fault & VM_FAULT_SIGSEGV) goto bad_area;" And then some architectures had a different pattern, and I tried to fix it up as straightforwardly as possible, but I could easily have screwed up. Can people take a look? On Tue, Jan 27, 2015 at 12:36 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
Very annoying. The patch would look something like the attached - TOTALLY UNTESTED.
Actually, I missed a couple of places in mm/gup.c and mm/ksm.c (and one in lustre, although that one just uses filemap_fault, so it never triggers the stack case, but for completeness). So this would be the more complete patch. Still totally untested. I may have screwed up something obvious. Linus
On Tue, Jan 27, 2015 at 12:57:20PM -0800, Linus Torvalds wrote:
[ Adding 'linux-arch' to the recipients, since this touches pretty much all architectures ] [...] Can people take a look?
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index 811937bb90be..9065d5aa3932 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -374,6 +374,12 @@ static noinline void do_fault_error(struct pt_regs *regs, int fault) do_no_context(regs); else pagefault_out_of_memory(); + } else if (fault & VM_FAULT_SIGSEGV) { + /* Kernel mode? Handle exceptions or die */ + if (!user_mode(regs)) + do_no_context(regs); + else + do_sigsegv(regs, SEGV_MAPERR);
s390 still compiles and boots with this patch applied. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 11:57 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
So this would be the more complete patch. Still totally untested. I may have screwed up something obvious.
Condition in arch/powerpc/mm/copro_fault.c hunk doesn't look right. -- Thanks. -- Max -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jan 28, 2015 at 10:59 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
Condition in arch/powerpc/mm/copro_fault.c hunk doesn't look right.
Yes. There was a typo too ("faulr" instead of "fault" in one arch). I didn't bother sending out my minor updated version because my fixes were tiny, but since somebody actually went through the old version, here is my updated one. Linus
On Thu, 2015-01-29 at 10:16 -0800, Linus Torvalds wrote:
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index eb79907f34fa..6154b0a2b063 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -437,6 +437,8 @@ good_area: */ fault = handle_mm_fault(mm, vma, address, flags); if (unlikely(fault & (VM_FAULT_RETRY|VM_FAULT_ERROR))) { + if (fault & VM_FAULT_SIGSEGV) + goto bad_area; rc = mm_fault_error(regs, address, fault); if (rc >= MM_FAULT_RETURN) goto bail;
I prefer having the test inside mm_fault_error(), even if that makes the patch a bit bigger, it keeps the logic in a single place. Untested patch: --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -184,6 +184,12 @@ static int mm_fault_error(struct pt_regs *regs, unsigned long addr, int fault) return MM_FAULT_RETURN; } + /* Other faults */ + + if (fault & VM_FAULT_SIGSEGV) { + up_read(¤t->mm->mmap_sem); + return MM_FAULT_ERR(SIGSEGV); + } if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON|VM_FAULT_HWPOISON_LARGE)) return do_sigbus(regs, addr, fault); Cheers, Ben. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Sun, Feb 1, 2015 at 4:23 PM, Benjamin Herrenschmidt <benh@au1.ibm.com> wrote:
I prefer having the test inside mm_fault_error(), even if that makes the patch a bit bigger, it keeps the logic in a single place. Untested patch:
I'm certainly ok with that, but I wanted to make the code that I wasn't going to compile (much less test) for various architectures be as simple and straightforward as possible. So feel free to send a patch that fixes it up to do it in a single place after testing it. Of course, what I *really* want would be to make a new "generic_mm_fault()" helper that would do all the normal stuff: - find_vma() - check permissions and ranges - call 'handle_mm_fault()' - do the proper error, retry and minor/major fault handling and then most architectures could just call that. Everybody has their own thing that they do *before* they get the mmap semaphore (x86 has at least kprobe, mmio tracing, vmalloc-fault, spurious prefetch faults due to AMD CPU bugs, in-atomic debugging irq re-enablement, etc etc). Those aren't even worth it trying to make into generic things. But just looking at the original patch, a _lot_ of the stuff around the actual handle_mm_fault() call is very very generic indeed, and I think it could be made into a generic helper function. For example, I not that long ago fixed the x86 handling of VM_FAULT_RETRY when there was a fatal signal pending and we were returning to kernel mode. The code would just return, "knowing" that the fatal signal would mean that rather than faulting again, the process would be killed. Except if the fault happened from kernel space, that wouldn't happen, and it really would just fault forever. I would not be surprised if somebody copied the old incorrect x86 case for VM_FAULT_RETRY and fatal signals, and it just never got fixed anywhere else. Doing all that in a generic helper routine (that then just returns a value saying "SIGBUS", "SIGSEGV" - we'd probably have to split it up into "SIGSEGV due to access error" vs "SIGSEGV due to map error" to make everybody happy, "out of memory" or "success" would have made it trivial to add the whole VM_FAULT_SIGSEGV, but it would also mean that everybody got fixes to the retry handling etc. Because right now there's a *lot* of basically duplicated code (although powerpc and s390 in particular have made some changes of their own). Anybody willing to see if they could encapsulate that part of the x86 code, and make it more widely useful? I say "x86 code", because that's the most tested one, and I think it gets the odd retry and error cases right (and minor/major fault counting etc), unlike some. Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Sun, 2015-02-01 at 17:09 -0800, Linus Torvalds wrote:
On Sun, Feb 1, 2015 at 4:23 PM, Benjamin Herrenschmidt <benh@au1.ibm.com> wrote:
I prefer having the test inside mm_fault_error(), even if that makes the patch a bit bigger, it keeps the logic in a single place. Untested patch:
I'm certainly ok with that, but I wanted to make the code that I wasn't going to compile (much less test) for various architectures be as simple and straightforward as possible.
Ah, I missed your reply ... my fault for using the wrong email address to send my message in the first place :-)
So feel free to send a patch that fixes it up to do it in a single place after testing it.
Ok sure, I'll have a look in the next few days, bogged down with some local emergency right now.
Of course, what I *really* want would be to make a new "generic_mm_fault()" helper that would do all the normal stuff:
- find_vma() - check permissions and ranges - call 'handle_mm_fault()' - do the proper error, retry and minor/major fault handling
and then most architectures could just call that.
That would be great ...
Anybody willing to see if they could encapsulate that part of the x86 code, and make it more widely useful? I say "x86 code", because that's the most tested one, and I think it gets the odd retry and error cases right (and minor/major fault counting etc), unlike some.
I can try to give it a spin some time this week I think, I can probably do x86, powerpc and arm. Let's see if I manage to not forget :) Cheers, Ben.
Linus -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tuesday 2015-01-27 21:36, Linus Torvalds wrote:
The only reason it returns SIGBUS is that we don't have an equivalent VM_FAULT_SIGSEGV error return. Which is something of an oversight, but adding it is a pain too, simply because the callers are mainly the architecture page fault handlers, so the VM_FAULT_SIGSEGV handling has to be added to all of them.
Very annoying. The patch would look something like the attached - TOTALLY UNTESTED.
Does this fix things for you?
The 2nd patch (from 12:57:20 -0800) plus the one-liner for mm/memory.c at the callsite for check_stack_guard_page makes the test still not succeed. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 1:12 PM, Jan Engelhardt <jengelh@inai.de> wrote:
The 2nd patch (from 12:57:20 -0800) plus the one-liner for mm/memory.c at the callsite for check_stack_guard_page makes the test still not succeed.
Really? What platform? I get ================== All 5 tests passed ================== with the patch I sent out, combined with this: diff --git a/mm/memory.c b/mm/memory.c index 54f3a9b00956..2c3536cc6c63 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2632,7 +2632,7 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma, /* Check if we need to add a guard page to the stack */ if (check_stack_guard_page(vma, address) < 0) - return VM_FAULT_SIGBUS; + return VM_FAULT_SIGSEGV; /* Use the zero-page for reads */ if (!(flags & FAULT_FLAG_WRITE) && !mm_forbids_zeropage(mm)) { but since architecture matters, and I run on x86-64, some other platform might still have issues. (And yes, I checked that it failed without the patch, so I think I'm testing the same thing you guys are) Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tuesday 2015-01-27 22:32, Linus Torvalds wrote:
On Tue, Jan 27, 2015 at 1:12 PM, Jan Engelhardt <jengelh@inai.de> wrote:
The 2nd patch (from 12:57:20 -0800) plus the one-liner for mm/memory.c at the callsite for check_stack_guard_page makes the test still not succeed.
Really? What platform? I get "all 5 tests passed" with the patch I sent out, combined with this: diff --git a/mm/memory.c b/mm/memory.c index 54f3a9b00956..2c3536cc6c63 100644
Turns out I missed a make call in the procedure *blush*. Sometimes I wish `make modules_install` would run the compile stage as well.. just like `make install` does in autotooled projects. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jan 27, 2015 at 2:14 PM, Jan Engelhardt <jengelh@inai.de> wrote:
Sometimes I wish `make modules_install` would run the compile stage as well.. just like `make install` does in autotooled projects.
This is just one more reason to hate autotools. It's shit. You should *never* compile things as root. It's not just a bad idea in that it leaves possible root-owned files in your project (which in turn can make cleanup harder etc), but it is historically also an actual security issue with the whole temporary file mess in /tmp and various racy symlink-attacks. So in general, you should try to teach people that "make install" should never ever build anything. Because if it does, it's just a broken code-flow. It may work, it may be convenient, and with proper tools it _may_ even be secure, but it's still a really bad idea. Sadly, I seem to be alone in my hatred of automake/configure/etc. Linus -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tuesday 2015-01-27 23:32, Linus Torvalds wrote:
On Tue, Jan 27, 2015 at 2:14 PM, Jan Engelhardt <jengelh@inai.de> wrote:
Sometimes I wish `make modules_install` would run the compile stage as well.. just like `make install` does in autotooled projects.
You should *never* compile things as root.
I certainly agree with you on that. So perhaps make throwing an error instead would be helpful here - "hey there are uncompiled things left, modules_install would give you nonmatching files".
Sadly, I seem to be alone in my hatred of automake/configure/etc.
(Look no further than netdev if you need company.) There are certainly points where it does not excel; however, in a distribution where we have to deal with not just one idiosyncratic build system, but 15+, it is the preferred choice to have especially if and when it comes down having to edit the build instructions for one reason or another. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
From: Linus Torvalds <torvalds@linux-foundation.org> Date: Tue, 27 Jan 2015 14:32:11 -0800
Sadly, I seem to be alone in my hatred of automake/configure/etc.
You're not, there are even I Hate Autoconf t-shirts. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Linus Torvalds <torvalds@linux-foundation.org> writes:
So in general, you should try to teach people that "make install" should never ever build anything. Because if it does, it's just a broken code-flow. It may work, it may be convenient, and with proper tools it _may_ even be secure, but it's still a really bad idea.
# make modules_install Makefile:657: Cannot use CONFIG_CC_STACKPROTECTOR_REGULAR: -fstack-protector not supported by compiler [...] Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
participants (11)
-
Andreas Schwab -
Benjamin Herrenschmidt -
David Miller -
Dimstar / Dominique Leuenberger -
Heiko Carstens -
Jan Engelhardt -
Linus Torvalds -
Max Filippov -
Michal Marek -
Takashi Iwai -
Yamaban