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 49399C433EF for ; Fri, 28 Jan 2022 15:52:14 +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=RlNbA+/1ckOssT9yQYcNQrV6zLu5F+Ak8mqHBWcV9wk=; b=GSpJwVBy00Mbfi 3Eoi8HMOkJkrEOJLY/ULqxhVC/QrM7lFzY2NRb6VmQd3apew3O8AScVX9Rf3q5oxQoYhQNHN6qnwK fz/Tgmk9O19M1EgIaTRwIPoSg8viGfXR6JKvtYaagN3G7PcALZ6kZSg/L5GYHDAG9Rx3yImLkrXPj 2jncIa0rg6IsNZm58iop9AKt18xqniV6NNBh91ANuMg7xR5o9lBrWHV5qfSKFa8/m+6KLomP5KtLY haSyW7acT8PPzNTNN84J0oW9AgQJmIqNOH0tpkCepxr1t8dEMr/dKjnlXEg7kAxw2O9cF29BzQYFA GXXs+pBzRq8kB2apActA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDTWi-002mXY-Q6; Fri, 28 Jan 2022 15:51:04 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nDTWe-002mWg-Ct for linux-arm-kernel@lists.infradead.org; Fri, 28 Jan 2022 15:51:02 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 8AFAFB803F5; Fri, 28 Jan 2022 15:50:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9C049C340E6; Fri, 28 Jan 2022 15:50:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1643385057; bh=dHqKFVDo8/TjjOkINPkqqECV8kHJmc9ubDxhkDsKCRA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UIgn+uYaIcZnrR3GGRYpPs9PW6Z1jWKIYs0QDXBkwYkumMfftCcyJFOPvS1SHGgY9 TSbIG/p3kqKO1Xu80pNP/vh0TLex+Fw7YNdDY+AGe0c2DzHy2kwWTRnbZKzRJCUwCz KayXZKOHERpEOvHwSeSMCCTuIGIGuH3j1ATxeaOs= Date: Fri, 28 Jan 2022 16:50:54 +0100 From: Greg KH To: Adam Ford Cc: Zhou Qingyang , kjlu@umn.edu, Abel Vesa , Michael Turquette , Stephen Boyd , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , linux-clk , arm-soc , Linux Kernel Mailing List Subject: Re: [PATCH] clk: imx: Fix a NULL pointer dereference in imx_register_uart_clocks() Message-ID: References: <20220124165206.55059-1-zhou1615@umn.edu> 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-20220128_075100_767927_8477B5D2 X-CRM114-Status: GOOD ( 40.03 ) 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 Fri, Jan 28, 2022 at 07:47:06AM -0600, Adam Ford wrote: > On Fri, Jan 28, 2022 at 4:16 AM Greg KH wrote: > > > > On Tue, Jan 25, 2022 at 12:52:06AM +0800, Zhou Qingyang wrote: > > > In imx_register_uart_clocks(), the global variable imx_uart_clocks is > > > assigned by kcalloc() and there is a dereference of in the next for loop, > > > which could introduce a NULL pointer dereference on failure of kcalloc(). > > > > > > Fix this by adding a NULL check of imx_uart_clocks. > > > > > > This bug was found by a static analyzer. > > > > > > Builds with 'make allyesconfig' show no new warnings, > > > and our static analyzer no longer warns about this code. > > > > > > Fixes: 379c9a24cc23 ("clk: imx: Fix reparenting of UARTs not associated with stdout") > > > Signed-off-by: Zhou Qingyang > > > --- > > > The analysis employs differential checking to identify inconsistent > > > security operations (e.g., checks or kfrees) between two code paths > > > and confirms that the inconsistent operations are not recovered in the > > > current function or the callers, so they constitute bugs. > > > > > > Note that, as a bug found by static analysis, it can be a false > > > positive or hard to trigger. Multiple researchers have cross-reviewed > > > the bug. > > > > > > drivers/clk/imx/clk.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c > > > index 7cc669934253..99249ab361d2 100644 > > > --- a/drivers/clk/imx/clk.c > > > +++ b/drivers/clk/imx/clk.c > > > @@ -173,6 +173,8 @@ void imx_register_uart_clocks(unsigned int clk_count) > > > int i; > > > > > > imx_uart_clocks = kcalloc(clk_count, sizeof(struct clk *), GFP_KERNEL); > > > + if (!imx_uart_clocks) > > > + return; > > > > > > if (!of_stdout) > > > return; > > > -- > > > 2.25.1 > > > > > > > As stated before, umn.edu is still not allowed to contribute to the > > Linux kernel. Please work with your administration to resolve this > > issue. > > Greg, > > In the interest of safety, I believe this patch is reasonable. How can kcalloc really fail here? Seriously, this is an impossible thing to happen in real-world situations, you have to have special fault-injection tooling to ever hit this in a system that is not just frozen due to other problems. > I > wrote the original patch that is being fixed by this. Would it be > acceptable if I submitted the same patch with "suggested-by" > associated with Zhou @ umn.edu? I want to give credit where credit is > due while still maintaining the rule that patches are not actually > being accepted by umn.edu. If you think this really is needed, then yes, feel free to rewrite it. But rewrite it to be correct. As it is, this is not correct. If an error happens because we are out of memory, actually handle that and do not just return as if everything worked properly like this patch is doing here. The "suggestion" here is incorrect, which is the big problem here. Whatever tool this group is using is wrong, and as a few people have hinted to me offline, maybe they are just still messing around with us and seeing how we behave. Personally, I'm starting to get mad. thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel