From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756795AbdDQPji (ORCPT ); Mon, 17 Apr 2017 11:39:38 -0400 Received: from mga11.intel.com ([192.55.52.93]:51334 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754696AbdDQPiE (ORCPT ); Mon, 17 Apr 2017 11:38:04 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,215,1488873600"; d="scan'208";a="957860470" Subject: Re: [PATCH] x86/mpx: Correctly report do_mpx_bt_fault() failures to user-space To: Joerg Roedel , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" References: <1491488362-27198-1-git-send-email-joro@8bytes.org> Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Joerg Roedel From: Dave Hansen Message-ID: <0d387d7f-208e-75aa-55ea-0157412aa4d4@linux.intel.com> Date: Mon, 17 Apr 2017 08:38:03 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1491488362-27198-1-git-send-email-joro@8bytes.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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.