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 08072D1CDCF for ; Tue, 22 Oct 2024 10:07:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id: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-Owner; bh=rDNfhvCyWnIJtBt/cVyjCF1IKMrba30fvxrKzlQyRNs=; b=f2C2odX1ZPRD80ToeU2LY5is/5 IC1+RMrdv/u8rM31mtMxAk5ytKSyUtt22TDjpVvG4jnaY0MhVeo4PAg8oOIm2WEILf7JkzS21VBCO REkYF93+i7iLp31uvHMVGoWmPXLlNRJ/MLaayIobNBMK7XJ+5JrdNIdw61fYG9drUAnwB7OKP8Eib PFI7B5+qs/lQ92tA+ksvNTb/Z5Rnrp3P8NjeQ7PuRHWsSJK/PAes3SyOkW1rlkvf5hYnf61HsfVIZ /u7G7b3j8NlO2HWKhtxILmx73E550ZEqCnOnrqyJftSzViJSqPv2fqywx0u04R2VPUYSlPgGeGvQC 3SIFBYMQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t3Bmm-0000000ATO5-1wuC; Tue, 22 Oct 2024 10:06:44 +0000 Received: from mail.alien8.de ([2a01:4f9:3051:3f93::2]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t3BS0-0000000AP6Q-3ZqJ for linux-arm-kernel@lists.infradead.org; Tue, 22 Oct 2024 09:45:18 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTP id CE43640E0219; Tue, 22 Oct 2024 09:45:02 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at mail.alien8.de Authentication-Results: mail.alien8.de (amavisd-new); dkim=pass (4096-bit key) header.d=alien8.de Received: from mail.alien8.de ([127.0.0.1]) by localhost (mail.alien8.de [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id llMr0-Ydhu4W; Tue, 22 Oct 2024 09:44:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=alien8; t=1729590298; bh=rDNfhvCyWnIJtBt/cVyjCF1IKMrba30fvxrKzlQyRNs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dZT4i07Sd+NwwdlM4Axx/COyVdbGqXuEhvR6he0hLYxXRFe0FIaYuiMD7NRITkFBW N97SKZCxFfkoD5+2izcSf4d4x7g+JKQNhA4kiw+Ro50bMczvm7S3XiMLp2n83pUvPI 1AOTD523p78tUkdxbO9KdoL35Q0F2og8hsDmzYCanCmsKgVUmASpio0wMn8C0fDhIi LN/OD3IpSO1gOtRYUFKc1Sdl1WM0vcRZbOranUmqvxz01pXPmtC3h56Grso7sYfX5w HH8FBLE9ppoiq0yQHQPNHdXfsQ1o4U2Jd220koiACcuP0bchUe4iszoI81ugsclF+k QW4jKmfjCnGfws8gCIBjuEUzu1VSppMzSggiigf/1gD3kVGIUHfBe0IlcViVUGUnYh iFlnm0Lm0z12q8OD7TeCX8thTlzh5hmcXPE4N23uCAopTBXIGZdR9A8BCG81qKC0eC on39IgcW8yzdN6hcZkeDldE1/DzOx8/d/Ifzism909UUEsMqu5+NG1ieCUlAR11IDU XvHRjlP3NWKhvsepBCPKUwwumeY8neN8LPfJAB9jop8dfbWPp0nEYRATRQmb3DZ5IG KkgcKcavZ1Yx9vn24jymDLwIHszHC4Ies1z1CWWV6Qsda/rzVUDoVMoq2SN9yA3hmF Rw2YyJhryeh2WiIZVZ5KDEPk= Received: from zn.tnic (p5de8e8eb.dip0.t-ipconnect.de [93.232.232.235]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-256) server-digest SHA256) (No client certificate requested) by mail.alien8.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id C3E4940E015F; Tue, 22 Oct 2024 09:44:34 +0000 (UTC) Date: Tue, 22 Oct 2024 11:44:29 +0200 From: Borislav Petkov To: Frank Li Cc: York Sun , Tony Luck , James Morse , Mauro Carvalho Chehab , Robert Richter , Krzysztof Kozlowski , Rob Herring , Conor Dooley , Krzysztof Kozlowski , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org, Borislav Petkov , devicetree@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, Priyanka Singh , Sherry Sun , Li Yang Subject: Re: [PATCH v3 3/6] EDAC/fsl_ddr: Fix bad bit shift operations Message-ID: <20241022094429.GFZxdz_QNHHr_DCPp3@fat_crate.local> References: <20241016-imx95_edac-v3-0-86ae6fc2756a@nxp.com> <20241016-imx95_edac-v3-3-86ae6fc2756a@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20241016-imx95_edac-v3-3-86ae6fc2756a@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241022_024517_058351_16349ACF X-CRM114-Status: GOOD ( 23.12 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Oct 16, 2024 at 04:31:11PM -0400, Frank Li wrote: > From: Priyanka Singh > > Fix undefined behavior caused by left-shifting a negative value in the > expression: > > cap_high ^ (1 << (bad_data_bit - 32)) > > The variable 'bad_data_bit' ranges from 0 to 63. When 'bad_data_bit' is > less than 32, 'bad_data_bit - 32' becomes negative, and left-shifting by a > negative value in C is undefined behavior. > > Fix this by combining 'cap_high' and 'cap_low' into a 64-bit variable. > > Fixes: ea2eb9a8b620 ("EDAC, fsl-ddr: Separate FSL DDR driver from MPC85xx") > Signed-off-by: Priyanka Singh > Reviewed-by: Sherry Sun You can't keep Reviewed-by tags when you change a patch considerably: Documentation/process/submitting-patches.rst > Signed-off-by: Li Yang What does that SOB tag mean? > Signed-off-by: Frank Li > --- > drivers/edac/fsl_ddr_edac.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c > index 7a9fb1202f1a0..846a4ba25342a 100644 > --- a/drivers/edac/fsl_ddr_edac.c > +++ b/drivers/edac/fsl_ddr_edac.c > @@ -328,6 +328,9 @@ static void fsl_mc_check(struct mem_ctl_info *mci) > * TODO: Add support for 32-bit wide buses > */ > if ((err_detect & DDR_EDE_SBE) && (bus_width == 64)) { > + u64 cap = (u64)cap_high << 32 | (u64)cap_low; > + u32 s = syndrome; > + > sbe_ecc_decode(cap_high, cap_low, syndrome, > &bad_data_bit, &bad_ecc_bit); > > @@ -338,11 +341,15 @@ static void fsl_mc_check(struct mem_ctl_info *mci) > fsl_mc_printk(mci, KERN_ERR, > "Faulty ECC bit: %d\n", bad_ecc_bit); > > + if (bad_data_bit >= 0) >= 0 implies != -1, right? IOW? diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c index 846a4ba25342..fe822cb9b562 100644 --- a/drivers/edac/fsl_ddr_edac.c +++ b/drivers/edac/fsl_ddr_edac.c @@ -328,24 +328,21 @@ static void fsl_mc_check(struct mem_ctl_info *mci) * TODO: Add support for 32-bit wide buses */ if ((err_detect & DDR_EDE_SBE) && (bus_width == 64)) { - u64 cap = (u64)cap_high << 32 | (u64)cap_low; + u64 cap = (u64)cap_high << 32 | cap_low; u32 s = syndrome; sbe_ecc_decode(cap_high, cap_low, syndrome, &bad_data_bit, &bad_ecc_bit); - if (bad_data_bit != -1) - fsl_mc_printk(mci, KERN_ERR, - "Faulty Data bit: %d\n", bad_data_bit); - if (bad_ecc_bit != -1) - fsl_mc_printk(mci, KERN_ERR, - "Faulty ECC bit: %d\n", bad_ecc_bit); - - if (bad_data_bit >= 0) + if (bad_data_bit >= 0) { + fsl_mc_printk(mci, KERN_ERR, "Faulty Data bit: %d\n", bad_data_bit); cap ^= 1ULL << bad_data_bit; + } - if (bad_ecc_bit >= 0) + if (bad_ecc_bit >= 0) { + fsl_mc_printk(mci, KERN_ERR, "Faulty ECC bit: %d\n", bad_ecc_bit); s ^= 1 << bad_ecc_bit; + } fsl_mc_printk(mci, KERN_ERR, "Expected Data / ECC:\t%#8.8x_%08x / %#2.2x\n", -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette