Received: by 10.192.165.148 with SMTP id m20csp831731imm; Thu, 10 May 2018 01:04:14 -0700 (PDT) X-Google-Smtp-Source: AB8JxZp2cfOrSUCpeF2A4jIFBjqMgTXfzkWN4wUtKiDvkQFXrmcEbSzZNTVoaa5KIPB6IqeSBORI X-Received: by 2002:a63:7208:: with SMTP id n8-v6mr320796pgc.420.1525939454754; Thu, 10 May 2018 01:04:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1525939454; cv=none; d=google.com; s=arc-20160816; b=hTBUj/qzAgpAY2aZYWr4b0c3JfIXMeFWFh5+D5pHm37Q3pXl5ZuKyVwvHCmxqXDamw Chq5gw4NjjOLGTVq5pKyEI74ODg9Nxxcxjach6aAA/AtYwnIDOIs6MeaGpnVYOOojNEt 0/j0sEv89IkwnOdnCY6fRIL3dF0iPAuGxfvmsQxWD4ROfyIhQLO2k2zXyrwSoyOJ1V/L XbdK3sUVmCxbEVGXjE6hxyKY7/ebbFneSqpVBJCNqdWGIMyAXZ/iZWVaf7TUJc8cpunz /38Ye29DHs5xvtxBITQWRgVVKftel8fdC59b8CTkMRyROX67VsMp4o18qEyrwh6iY03R IPLQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=ns1LkFwmgIN9PqQxhb8E3xiDbglTB7NEUxT792msWj4=; b=zdMotO+cYUCtmDjoGUMhO03/+qKFXh0lgYvSXMgX56MR0efuUixjq7y1v57989i7HT K/wo5ZxUuTvoPfeQGpvTa2balXS5NsYwLckPBww0+JGyLgPYJr7n/dGc4kbdjEfEmfHK ReZ0xQ231wc6Y4bSN2MaTeYbugFOXxYNGPXcqf+Bk4gnVjlapWNNPd4qK8ArxphqLB8h yku0hY1IF6DDgZHW0EsaR8Xz5VaVesX15RVaLebm7bdm1l0q7i+wKPVz9msMzQgz8m3o DJDu9Fa8zrib/YuUB+d9zEztpWkgVcpc1SkdCEKh8HpgT1hvgl1WQ6yS+AHHJnSaQirR iAnA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id r1-v6si232578plo.414.2018.05.10.01.03.59; Thu, 10 May 2018 01:04:14 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756699AbeEJIDY (ORCPT + 99 others); Thu, 10 May 2018 04:03:24 -0400 Received: from 9pmail.ess.barracuda.com ([64.235.150.225]:53274 "EHLO 9pmail.ess.barracuda.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753696AbeEJIDU (ORCPT ); Thu, 10 May 2018 04:03:20 -0400 Received: from mipsdag02.mipstec.com (mail2.mips.com [12.201.5.32]) by mx27.ess.sfj.cudaops.com (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA256 bits=128 verify=NO); Thu, 10 May 2018 07:59:03 +0000 Received: from [192.168.155.41] (192.168.155.41) by mipsdag02.mipstec.com (10.20.40.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1415.2; Thu, 10 May 2018 00:59:28 -0700 Subject: Re: [REVIEW][PATCH 08/22] signal/mips: Use force_sig_fault where appropriate To: "Eric W. Biederman" CC: , , Ralf Baechle , James Hogan , References: <87604mhrnb.fsf@xmission.com> <20180420143811.9994-8-ebiederm@xmission.com> <8736z0s087.fsf@xmission.com> From: Matt Redfearn Message-ID: <6811e06d-ac0d-35a6-7d86-57838d5d7f8e@mips.com> Date: Thu, 10 May 2018 08:59:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <8736z0s087.fsf@xmission.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [192.168.155.41] X-ClientProxiedBy: mipsdag02.mipstec.com (10.20.40.47) To mipsdag02.mipstec.com (10.20.40.47) X-BESS-ID: 1525939142-637137-8295-345664-1 X-BESS-VER: 2018.5-r1804261738 X-BESS-Apparent-Source-IP: 12.201.5.32 X-BESS-Outbound-Spam-Score: 0.50 X-BESS-Outbound-Spam-Report: Code version 3.2, rules version 3.2.2.192870 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------- 0.50 BSF_RULE7568M META: Custom Rule 7568M 0.00 BSF_BESS_OUTBOUND META: BESS Outbound X-BESS-Outbound-Spam-Status: SCORE=0.50 using account:ESS59374 scores of KILL_LEVEL=7.0 tests=BSF_RULE7568M, BSF_BESS_OUTBOUND X-BESS-BRTS-Status: 1 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Eric, On 10/05/18 03:39, Eric W. Biederman wrote: > Matt Redfearn writes: > >> Hi Eric, >> >> On 20/04/18 15:37, Eric W. Biederman wrote: >>> Filling in struct siginfo before calling force_sig_info a tedious and >>> error prone process, where once in a great while the wrong fields >>> are filled out, and siginfo has been inconsistently cleared. >>> >>> Simplify this process by using the helper force_sig_fault. Which >>> takes as a parameters all of the information it needs, ensures >>> all of the fiddly bits of filling in struct siginfo are done properly >>> and then calls force_sig_info. >>> >>> In short about a 5 line reduction in code for every time force_sig_info >>> is called, which makes the calling function clearer. >>> >>> Cc: Ralf Baechle >>> Cc: James Hogan >>> Cc: linux-mips@linux-mips.org >>> Signed-off-by: "Eric W. Biederman" >>> --- >>> arch/mips/kernel/traps.c | 65 ++++++++++++++---------------------------------- >>> arch/mips/mm/fault.c | 19 ++++---------- >>> 2 files changed, 23 insertions(+), 61 deletions(-) >>> >>> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c >>> index 967e9e4e795e..66ec4b0b484d 100644 >>> --- a/arch/mips/kernel/traps.c >>> +++ b/arch/mips/kernel/traps.c >>> @@ -699,17 +699,11 @@ static int simulate_sync(struct pt_regs *regs, unsigned int opcode) >>> asmlinkage void do_ov(struct pt_regs *regs) >>> { >>> enum ctx_state prev_state; >>> - siginfo_t info; >>> - >>> - clear_siginfo(&info); >>> - info.si_signo = SIGFPE; >>> - info.si_code = FPE_INTOVF; >>> - info.si_addr = (void __user *)regs->cp0_epc; >>> prev_state = exception_enter(); >>> die_if_kernel("Integer overflow", regs); >>> - force_sig_info(SIGFPE, &info, current); >>> + force_sig_fault(SIGFPE, FPE_INTOVF, (void __user *)regs->cp0_epc, current); >>> exception_exit(prev_state); >>> } >>> @@ -722,32 +716,27 @@ asmlinkage void do_ov(struct pt_regs *regs) >>> void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, >>> struct task_struct *tsk) >>> { >>> - struct siginfo si; >>> - >>> - clear_siginfo(&si); >>> - si.si_addr = fault_addr; >>> - si.si_signo = SIGFPE; >>> + int si_code; >> >> This is giving build errors in Linux next >> (https://storage.kernelci.org/next/master/next-20180509/mips/defconfig+kselftest/build.log) >> >> si_code would have ended up as 0 before from the clear_siginfo(), but perhaps > > And si_code 0 is not a valid si_code to use with a floating point > siginfo layout. > >> int si_code = FPE_FLTUNK; >> >> Would make a more sensible default? > > FPE_FLTUNK would make a more sensible default. > > I seem to remember someone telling me that case can never happen in > practice so I have simply not worried about it. Perhaps I am > misremembering this. It probably can't happen in practise - but the issue is that the kernel doesn't even compile because -Werror=maybe-uninitialized results in a build error since the compiler can't know that one of the branches will definitely be taken to set si_code. Thanks, Matt > > Eric >