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 <jroe...@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.

Reply via email to