* [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
@ 2017-04-06 14:19 Joerg Roedel
2017-04-12 7:30 ` [tip:x86/mm] " tip-bot for Joerg Roedel
2017-04-17 15:38 ` [PATCH] " Dave Hansen
0 siblings, 2 replies; 7+ messages in thread
From: Joerg Roedel @ 2017-04-06 14:19 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
Cc: x86, Dave Hansen, linux-kernel, Joerg Roedel
From: Joerg Roedel <jroedel@suse.de>
When this function fails it just sends a SIGSEGV signal to
user-space using force_sig(). This signal is missing
essential information about the cause, e.g. the trap_nr or
an error code.
Fix this by propagating the error to the only caller of
mpx_handle_bd_fault(), do_bounds(), which sends the correct
SIGSEGV signal to the process.
Fixes: fe3d197f84319 ('x86, mpx: On-demand kernel allocation of bounds tables')
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
arch/x86/mm/mpx.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index cd44ae7..1c34b76 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -526,15 +526,7 @@ int mpx_handle_bd_fault(void)
if (!kernel_managing_mpx_tables(current->mm))
return -EINVAL;
- if (do_mpx_bt_fault()) {
- force_sig(SIGSEGV, current);
- /*
- * The force_sig() is essentially "handling" this
- * exception, so we do not pass up the error
- * from do_mpx_bt_fault().
- */
- }
- return 0;
+ return do_mpx_bt_fault();
}
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:x86/mm] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-06 14:19 [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space Joerg Roedel
@ 2017-04-12 7:30 ` tip-bot for Joerg Roedel
2017-04-17 15:38 ` [PATCH] " Dave Hansen
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Joerg Roedel @ 2017-04-12 7:30 UTC (permalink / raw)
To: linux-tip-commits
Cc: jpoimboe, dvlasenk, peterz, jroedel, tglx, brgerst, dave.hansen,
linux-kernel, hpa, luto, torvalds, bp, mingo
Commit-ID: 5ed386ec09a5d75bcf073967e55e895c2607a5c3
Gitweb: http://git.kernel.org/tip/5ed386ec09a5d75bcf073967e55e895c2607a5c3
Author: Joerg Roedel <jroedel@suse.de>
AuthorDate: Thu, 6 Apr 2017 16:19:22 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 12 Apr 2017 08:40:58 +0200
x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
When this function fails it just sends a SIGSEGV signal to
user-space using force_sig(). This signal is missing
essential information about the cause, e.g. the trap_nr or
an error code.
Fix this by propagating the error to the only caller of
mpx_handle_bd_fault(), do_bounds(), which sends the correct
SIGSEGV signal to the process.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: fe3d197f84319 ('x86, mpx: On-demand kernel allocation of bounds tables')
Link: http://lkml.kernel.org/r/1491488362-27198-1-git-send-email-joro@8bytes.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/mm/mpx.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index cd44ae7..1c34b76 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -526,15 +526,7 @@ int mpx_handle_bd_fault(void)
if (!kernel_managing_mpx_tables(current->mm))
return -EINVAL;
- if (do_mpx_bt_fault()) {
- force_sig(SIGSEGV, current);
- /*
- * The force_sig() is essentially "handling" this
- * exception, so we do not pass up the error
- * from do_mpx_bt_fault().
- */
- }
- return 0;
+ return do_mpx_bt_fault();
}
/*
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-06 14:19 [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space Joerg Roedel
2017-04-12 7:30 ` [tip:x86/mm] " tip-bot for Joerg Roedel
@ 2017-04-17 15:38 ` Dave Hansen
2017-04-20 12:08 ` Joerg Roedel
1 sibling, 1 reply; 7+ messages in thread
From: Dave Hansen @ 2017-04-17 15:38 UTC (permalink / raw)
To: Joerg Roedel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin
Cc: x86, linux-kernel, Joerg Roedel
Hi Joerg,
> When this function fails it just sends a SIGSEGV signal to
> user-space using force_sig(). This signal is missing
> essential information about the cause, e.g. the trap_nr or
> an error code.
>
> Fix this by propagating the error to the only caller of
> mpx_handle_bd_fault(), do_bounds(), which sends the correct
> SIGSEGV signal to the process.
Just to be clear, the thing you're calling "correct" is this do_trap(),
right?
do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
> Fixes: fe3d197f84319 ('x86, mpx: On-demand kernel allocation of bounds
tables')
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
> arch/x86/mm/mpx.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index cd44ae7..1c34b76 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -526,15 +526,7 @@ int mpx_handle_bd_fault(void)
> if (!kernel_managing_mpx_tables(current->mm))
> return -EINVAL;
>
> - if (do_mpx_bt_fault()) {
> - force_sig(SIGSEGV, current);
> - /*
> - * The force_sig() is essentially "handling" this
> - * exception, so we do not pass up the error
> - * from do_mpx_bt_fault().
> - */
> - }
> - return 0;
> + return do_mpx_bt_fault();
> }
do_mpx_bt_fault() can fail for a bunch of reasons:
* unexpected or invalid value in BNDCSR
* out of memory (physical or virtual)
* unresolvable fault walking/filling bounds tables
* !valid and non-empty bad entry in the bounds tables
This will end up sending a signal that *looks* like a X86_TRAP_BR for
all of those, including those that are not really bounds-related, like
unresolvable faults. We also don't populate enough information in the
siginfo that gets delivered for userspace to resolve the fault.
I'm not sure this patch is the right thing.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-17 15:38 ` [PATCH] " Dave Hansen
@ 2017-04-20 12:08 ` Joerg Roedel
2017-04-20 15:45 ` Dave Hansen
0 siblings, 1 reply; 7+ messages in thread
From: Joerg Roedel @ 2017-04-20 12:08 UTC (permalink / raw)
To: Dave Hansen
Cc: Joerg Roedel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel
Hi Dave,
On Mon, Apr 17, 2017 at 08:38:03AM -0700, Dave Hansen wrote:
> Just to be clear, the thing you're calling "correct" is this do_trap(),
> right?
>
> do_trap(X86_TRAP_BR, SIGSEGV, "bounds", regs, error_code, NULL);
Yes, because it signals the right trap_nr and error_code to user-space.
> do_mpx_bt_fault() can fail for a bunch of reasons:
> * unexpected or invalid value in BNDCSR
> * out of memory (physical or virtual)
> * unresolvable fault walking/filling bounds tables
> * !valid and non-empty bad entry in the bounds tables
>
> This will end up sending a signal that *looks* like a X86_TRAP_BR for
> all of those, including those that are not really bounds-related, like
> unresolvable faults. We also don't populate enough information in the
> siginfo that gets delivered for userspace to resolve the fault.
>
> I'm not sure this patch is the right thing.
The problem is, without this patch the trap_nr reported to user-space is
0, which maps to divide-by-zero. I think this is wrong, and since all
failure cases from do_mpx_bt_fault() can only happen in the #BR
exception handler, I think that reporting X86_TRAP_BR for all failure
cases is the right thing to do.
I don't know whether user-space (with this patch) already gets enough
information from do_trap() to handle all of the above cases, but it is a
step in the right direction.
Joerg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-20 12:08 ` Joerg Roedel
@ 2017-04-20 15:45 ` Dave Hansen
2017-04-21 12:19 ` Joerg Roedel
0 siblings, 1 reply; 7+ messages in thread
From: Dave Hansen @ 2017-04-20 15:45 UTC (permalink / raw)
To: Joerg Roedel
Cc: Joerg Roedel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel
On 04/20/2017 05:08 AM, Joerg Roedel wrote:
>> do_mpx_bt_fault() can fail for a bunch of reasons:
>> * unexpected or invalid value in BNDCSR
>> * out of memory (physical or virtual)
>> * unresolvable fault walking/filling bounds tables
>> * !valid and non-empty bad entry in the bounds tables
>>
>> This will end up sending a signal that *looks* like a X86_TRAP_BR for
>> all of those, including those that are not really bounds-related, like
>> unresolvable faults. We also don't populate enough information in the
>> siginfo that gets delivered for userspace to resolve the fault.
>>
>> I'm not sure this patch is the right thing.
>
> The problem is, without this patch the trap_nr reported to user-space is
> 0, which maps to divide-by-zero. I think this is wrong, and since all
> failure cases from do_mpx_bt_fault() can only happen in the #BR
> exception handler, I think that reporting X86_TRAP_BR for all failure
> cases is the right thing to do.
Urg, that does sound bogus.
How about doing X86_TRAP_PF? That would at least be consistent with
SIGBUS, which is probably the closest thing to a generic error code that
we have.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-20 15:45 ` Dave Hansen
@ 2017-04-21 12:19 ` Joerg Roedel
2017-04-21 14:30 ` Dave Hansen
0 siblings, 1 reply; 7+ messages in thread
From: Joerg Roedel @ 2017-04-21 12:19 UTC (permalink / raw)
To: Dave Hansen
Cc: Joerg Roedel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel
On Thu, Apr 20, 2017 at 08:45:28AM -0700, Dave Hansen wrote:
> How about doing X86_TRAP_PF? That would at least be consistent with
> SIGBUS, which is probably the closest thing to a generic error code that
> we have.
Correct me if I am wrong, but for SIGBUS this only happens in the
page-fault path, right? And this path is indeed entered on a #PF
exception.
I see no reason to lie to user-space about the trap_nr that caused the
SIGSEGV, especially since user-space software needs to be modified to
make use of MPX, including the signal handler. So there is no risk of
introducing any incompatibility or regression, no?
Joerg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space
2017-04-21 12:19 ` Joerg Roedel
@ 2017-04-21 14:30 ` Dave Hansen
0 siblings, 0 replies; 7+ messages in thread
From: Dave Hansen @ 2017-04-21 14:30 UTC (permalink / raw)
To: Joerg Roedel
Cc: Joerg Roedel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
linux-kernel
On 04/21/2017 05:19 AM, Joerg Roedel wrote:
> On Thu, Apr 20, 2017 at 08:45:28AM -0700, Dave Hansen wrote:
>> How about doing X86_TRAP_PF? That would at least be consistent with
>> SIGBUS, which is probably the closest thing to a generic error code that
>> we have.
> Correct me if I am wrong, but for SIGBUS this only happens in the
> page-fault path, right? And this path is indeed entered on a #PF
> exception.
It can happen to programs for tons of reasons. It definitely happens
outside page faults.
> I see no reason to lie to user-space about the trap_nr that caused the
> SIGSEGV, especially since user-space software needs to be modified to
> make use of MPX, including the signal handler. So there is no risk of
> introducing any incompatibility or regression, no?
I think it's pretty safe to change.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-04-21 14:30 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 14:19 [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space Joerg Roedel
2017-04-12 7:30 ` [tip:x86/mm] " tip-bot for Joerg Roedel
2017-04-17 15:38 ` [PATCH] " Dave Hansen
2017-04-20 12:08 ` Joerg Roedel
2017-04-20 15:45 ` Dave Hansen
2017-04-21 12:19 ` Joerg Roedel
2017-04-21 14:30 ` Dave Hansen
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.