From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 71D5EC433FE for ; Thu, 13 Oct 2022 10:52:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gcPKyfUsqCpG8fy9E7lazCNNl8ehLlZ8awbAi6kFU+I=; b=be+YS6YiWmoUz/ 2WMgRHMyVRz5mPc77kIS7OojoZksAahojGmFzEdKTJW9O9YwdSXC/3QQu6SSw9mlPE8IVETkZR+FS nkHx3AyS7OXE02TC2DWYlxB4iovzu734vzxzuiYVIiitASHXeGqjVE1WUCnC+DL1FlLHHW4/SWV9M wEQXNJezdbW8S1YzyjaWZo1q8bdOT94LT8QpguZYWknz41U/QWPRiJJWYBEBWuGyK+eVA2PgFYCyI OHJjolxIYvhO58vasJ3m9hX9CNkSn5IstMHeGltJ2WbQmq4mjdQt4r+KgW03E0btdeoztIBIf7IO7 VsGLdjrKrLMiTn8cYA9Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oivoc-00BTqc-TU; Thu, 13 Oct 2022 10:51:51 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oivoZ-00BTpI-Fv for linux-arm-kernel@lists.infradead.org; Thu, 13 Oct 2022 10:51:49 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=oMej+kzRa21sec7MJHHUVxNEkYjFrQ8I/aiCxja5qqc=; b=mQ1tqaHLCNwG9I3Oj3G+/re7Nu hsF6m5VRnw9doczmn8cdjSBLI54KumZ83s4Z+vsgdklscqNrkxCDjryG6Ppf+X7NXz1+b+gq2w3/A 9G2k0lu43KJeOjPYfe0JtZsknmQRG9KEWnzfiBCt7MbXNli1Y/K+1N/Cff+1JXONbh9oHSb9V6UKW HjbfADEYwEB3pYQ6sxuxy6JpCSVeRF60PrcBH3Dob+G3MfdniKNIJwag6ZiSCNU96smOUFyCaeAhz w7X4jFxArm6cvdH8prgrgLA4oyTJFUeUqCAJiDyuROLlGcMqjOAKK/2YbSC8TLpP+FySw3T0dgWEV CoRSW2Ig==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:34704) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oivoU-0008Fi-Vb; Thu, 13 Oct 2022 11:51:43 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1oivoS-0004i9-6c; Thu, 13 Oct 2022 11:51:40 +0100 Date: Thu, 13 Oct 2022 11:51:40 +0100 From: "Russell King (Oracle)" To: "Leizhen (ThunderTown)" Cc: Ard Biesheuvel , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] ARM: Fix some check warnings of tool sparse Message-ID: References: <20221010095346.1957-1-thunder.leizhen@huawei.com> <20221010095346.1957-2-thunder.leizhen@huawei.com> <1ab4c651-f0ab-1107-1784-ad255dabc33a@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221013_035147_693090_F71BBCCA X-CRM114-Status: GOOD ( 31.24 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Oct 11, 2022 at 10:29:58AM +0800, Leizhen (ThunderTown) wrote: > On 2022/10/10 19:06, Ard Biesheuvel wrote: > > On Mon, 10 Oct 2022 at 12:58, Leizhen (ThunderTown) > > wrote: > >> On 2022/10/10 18:20, Ard Biesheuvel wrote: > >>> On Mon, 10 Oct 2022 at 11:56, Zhen Lei wrote: > >>>> > >>>> Fix the following warnings: > >>>> warning: incorrect type in initializer (different address spaces) > >>>> expected unsigned short [noderef] __user *register __p > >>>> got unsigned short [usertype] * > >>>> warning: cast removes address space '__user' of expression > >>>> > >>>> Signed-off-by: Zhen Lei > >>>> --- > >>>> arch/arm/kernel/traps.c | 10 +++++----- > >>>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > >>>> index 20b2db6dcd1ced7..34aa80c09c508c1 100644 > >>>> --- a/arch/arm/kernel/traps.c > >>>> +++ b/arch/arm/kernel/traps.c > >>>> @@ -188,9 +188,9 @@ static void dump_instr(const char *lvl, struct pt_regs *regs) > >>>> } > >>>> } else { > >>>> if (thumb) > >>>> - bad = get_user(val, &((u16 *)addr)[i]); > >>>> + bad = get_user(val, &((u16 __user *)addr)[i]); > >>>> else > >>>> - bad = get_user(val, &((u32 *)addr)[i]); > >>>> + bad = get_user(val, &((u32 __user *)addr)[i]); > >>>> } > >>>> > >>>> if (!bad) > >>>> @@ -455,15 +455,15 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > >>>> if (processor_mode(regs) == SVC_MODE) { > >>>> #ifdef CONFIG_THUMB2_KERNEL > >>>> if (thumb_mode(regs)) { > >>>> - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > >>>> + instr = __mem_to_opcode_thumb16(((__force u16 *)pc)[0]); > >>> > >>> Shouldn't this be __user as well? (and below) > >> > >> unsigned int instr; > >> void __user *pc; > >> > >> The __user can clear the warning, but a new warning will be generated. > >> > >> instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); > >> ^new ^old > >> > >> arch/arm/kernel/traps.c:473:33: warning: dereference of noderef expression > >> > > > > This is because dereferencing a __user pointer is not permitted. > > > > So this code should be using get_kernel_nofault() here not a plain > > dereference of PC. So better to fix that properly instead of papering > > over it with a __force cast just to make sparse happy. > > How about: > @@ -451,9 +451,9 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) > asmlinkage void do_undefinstr(struct pt_regs *regs) > { > unsigned int instr; > - void __user *pc; > + void *pc; > > - pc = (void __user *)instruction_pointer(regs); > + pc = (void *)instruction_pointer(regs); > > if (processor_mode(regs) == SVC_MODE) { > #ifdef CONFIG_THUMB2_KERNEL > @@ -497,7 +497,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) > } > #endif > arm_notify_die("Oops - undefined instruction", regs, > - SIGILL, ILL_ILLOPC, pc, 0, 6); > + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); > } > NOKPROBE_SYMBOL(do_undefinstr) > > > The 'pc' may come from kernel or user. And I found that all the get_user() > calls have already done type casts to the pc, except arm_notify_die(). > I think the above changes are reasonable. If we're going to do that, lets do it properly - I think the above would need some __force usage to stop sparse complaining, whereas I don't think this will (untested): diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 3f468ac98592..827cbc022900 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -449,36 +449,45 @@ int call_undef_hook(struct pt_regs *regs, unsigned int instr) asmlinkage void do_undefinstr(struct pt_regs *regs) { unsigned int instr; - void __user *pc; + unsigned long pc; - pc = (void __user *)instruction_pointer(regs); + pc = instruction_pointer(regs); if (processor_mode(regs) == SVC_MODE) { -#ifdef CONFIG_THUMB2_KERNEL - if (thumb_mode(regs)) { - instr = __mem_to_opcode_thumb16(((u16 *)pc)[0]); + if (IS_ENABLED(CONFIG_THUMB2_KERNEL) && thumb_mode(regs)) { + u16 *tpc = (u16 *)pc; + + instr = __mem_to_opcode_thumb16(tpc[0]); if (is_wide_instruction(instr)) { u16 inst2; - inst2 = __mem_to_opcode_thumb16(((u16 *)pc)[1]); + + inst2 = __mem_to_opcode_thumb16(tpc[1]); instr = __opcode_thumb32_compose(instr, inst2); } - } else -#endif - instr = __mem_to_opcode_arm(*(u32 *) pc); + } else { + u32 *apc = (u32 *)pc; + + instr = __mem_to_opcode_arm(*apc); + } } else if (thumb_mode(regs)) { - if (get_user(instr, (u16 __user *)pc)) + u16 __user *tpc = (u16 __user *)pc; + + if (get_user(instr, tpc)) goto die_sig; instr = __mem_to_opcode_thumb16(instr); if (is_wide_instruction(instr)) { unsigned int instr2; - if (get_user(instr2, (u16 __user *)pc+1)) + if (get_user(instr2, tpc + 1)) goto die_sig; instr2 = __mem_to_opcode_thumb16(instr2); instr = __opcode_thumb32_compose(instr, instr2); } } else { - if (get_user(instr, (u32 __user *)pc)) + u32 __user *apc = (u32 __user *)pc; + + if (get_user(instr, apc)) goto die_sig; + instr = __mem_to_opcode_arm(instr); } @@ -495,7 +504,7 @@ asmlinkage void do_undefinstr(struct pt_regs *regs) } #endif arm_notify_die("Oops - undefined instruction", regs, - SIGILL, ILL_ILLOPC, pc, 0, 6); + SIGILL, ILL_ILLOPC, (void __user *)pc, 0, 6); } NOKPROBE_SYMBOL(do_undefinstr) -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel