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 X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C3D70C433F5 for ; Thu, 23 Sep 2021 14:12:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 992F960F48 for ; Thu, 23 Sep 2021 14:12:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241308AbhIWONm (ORCPT ); Thu, 23 Sep 2021 10:13:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44158 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241304AbhIWONl (ORCPT ); Thu, 23 Sep 2021 10:13:41 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6D39EC061574 for ; Thu, 23 Sep 2021 07:12:10 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mTPSD-0006kc-MA; Thu, 23 Sep 2021 16:12:01 +0200 Received: from mtr by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1mTPSA-0000L2-3T; Thu, 23 Sep 2021 16:11:58 +0200 Date: Thu, 23 Sep 2021 16:11:58 +0200 From: Michael Tretter To: Harini Katakam Cc: vkoul@kernel.org, romain.perier@gmail.com, allen.lkml@gmail.com, yukuai3@huawei.com, dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, harinikatakamlinux@gmail.com, michal.simek@xilinx.com, radhey.shyam.pandey@xilinx.com, shravya.kumbham@xilinx.com Subject: Re: [PATCH 1/4] dmaengine: zynqmp_dma: Typecast the variable to handle overflow Message-ID: <20210923141158.GA30905@pengutronix.de> References: <20210914082817.22311-1-harini.katakam@xilinx.com> <20210914082817.22311-2-harini.katakam@xilinx.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210914082817.22311-2-harini.katakam@xilinx.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 15:08:04 up 217 days, 16:31, 115 users, load average: 0.30, 0.52, 0.45 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: mtr@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: dmaengine@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org Hi Harini, On Tue, 14 Sep 2021 13:58:14 +0530, Harini Katakam wrote: > From: Shravya Kumbham > > In zynqmp_dma_alloc/free_chan_resources functions there is a > potential overflow in the below expressions. > > dma_alloc_coherent(chan->dev, (2 * chan->desc_size * > ZYNQMP_DMA_NUM_DESCS), > &chan->desc_pool_p, GFP_KERNEL); > > dma_free_coherent(chan->dev,(2 * ZYNQMP_DMA_DESC_SIZE(chan) * > ZYNQMP_DMA_NUM_DESCS), > chan->desc_pool_v, chan->desc_pool_p); > > The arguments desc_size and ZYNQMP_DMA_NUM_DESCS are 32 bit. Though > this overflow condition is not observed but it is a potential problem > in the case of 32-bit multiplication. Hence fix it by using typecast. > > Addresses-Coverity: Event overflow_before_widen. > Signed-off-by: Shravya Kumbham > Reviewed-by: Radhey Shyam Pandey Thanks for the patch. Your SoB is missing in this and the other patches of this series. > --- > drivers/dma/xilinx/zynqmp_dma.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c > index 5fecf5aa6e85..2d0eba25739d 100644 > --- a/drivers/dma/xilinx/zynqmp_dma.c > +++ b/drivers/dma/xilinx/zynqmp_dma.c > @@ -490,7 +490,8 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan) > } > > chan->desc_pool_v = dma_alloc_coherent(chan->dev, > - (2 * chan->desc_size * ZYNQMP_DMA_NUM_DESCS), > + ((size_t)(2 * chan->desc_size) * > + ZYNQMP_DMA_NUM_DESCS), Wouldn't it be easier to change the type of chan->desc_size to size_t? Maybe we could also just calculate the size of the descriptor pool during probe() to make the code more readable? I also noticed that there is the ZYNQMP_DMA_DESC_SIZE() macro, which is inconsistently used in the driver. Maybe you could cleanup this as well as you are at it? > &chan->desc_pool_p, GFP_KERNEL); > if (!chan->desc_pool_v) > return -ENOMEM; > @@ -677,7 +678,8 @@ static void zynqmp_dma_free_chan_resources(struct dma_chan *dchan) > zynqmp_dma_free_descriptors(chan); > spin_unlock_irqrestore(&chan->lock, irqflags); > dma_free_coherent(chan->dev, > - (2 * ZYNQMP_DMA_DESC_SIZE(chan) * ZYNQMP_DMA_NUM_DESCS), > + ((size_t)(2 * ZYNQMP_DMA_DESC_SIZE(chan)) * > + ZYNQMP_DMA_NUM_DESCS), With a pre-calculated descriptor pool size, recalculating the size here wouldn't be necessary anymore. Michael > chan->desc_pool_v, chan->desc_pool_p); > kfree(chan->sw_desc_pool); > pm_runtime_mark_last_busy(chan->dev); > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > 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 X-Spam-Level: X-Spam-Status: No, score=-16.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B271C433F5 for ; Thu, 23 Sep 2021 14:13:39 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 6865E61090 for ; Thu, 23 Sep 2021 14:13:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6865E61090 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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=hSp4WGI8/HeDiruZp/OtlwVt4/w2NQrVsNrZY8zko44=; b=JVQTiiq5eBsqRb HYuRZdfRDvXWwQFj6fx8vhC2srU1bmeCBWcqTDX4CbzA1lOCJxwIx5xC3pRf8L0Vt8myt/VV0GSTK ioiOY72juUx5ZnuzQdD0OcUHE0245W0LnBrFLzGJCslC0xAwmtPWzTyPMjwb0JJWbyCCUMRUtgRdM FqqF8OSZ6PZ1VxgxyEE7tRmb/ni2C8W7vhgb+pn/F2ehcEaYDjhqkh8kXRywH+7fCppQQCsyzC0mX A757yWI7RadjKwbqOrbbm+vo9rC6qDpI5IAA5XCSzAmPBntd7YTZcoGYnM4bBT38YhYVDUZpcU1T7 IDgptXXW02DlpFFEq1qg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mTPSR-00BmxB-Hy; Thu, 23 Sep 2021 14:12:16 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mTPSI-00Bmtv-OQ for linux-arm-kernel@lists.infradead.org; Thu, 23 Sep 2021 14:12:08 +0000 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mTPSD-0006kc-MA; Thu, 23 Sep 2021 16:12:01 +0200 Received: from mtr by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1mTPSA-0000L2-3T; Thu, 23 Sep 2021 16:11:58 +0200 Date: Thu, 23 Sep 2021 16:11:58 +0200 From: Michael Tretter To: Harini Katakam Cc: vkoul@kernel.org, romain.perier@gmail.com, allen.lkml@gmail.com, yukuai3@huawei.com, dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, harinikatakamlinux@gmail.com, michal.simek@xilinx.com, radhey.shyam.pandey@xilinx.com, shravya.kumbham@xilinx.com Subject: Re: [PATCH 1/4] dmaengine: zynqmp_dma: Typecast the variable to handle overflow Message-ID: <20210923141158.GA30905@pengutronix.de> References: <20210914082817.22311-1-harini.katakam@xilinx.com> <20210914082817.22311-2-harini.katakam@xilinx.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210914082817.22311-2-harini.katakam@xilinx.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 15:08:04 up 217 days, 16:31, 115 users, load average: 0.30, 0.52, 0.45 User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: mtr@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210923_071206_885420_10B48D2B X-CRM114-Status: GOOD ( 27.32 ) 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 Hi Harini, On Tue, 14 Sep 2021 13:58:14 +0530, Harini Katakam wrote: > From: Shravya Kumbham > > In zynqmp_dma_alloc/free_chan_resources functions there is a > potential overflow in the below expressions. > > dma_alloc_coherent(chan->dev, (2 * chan->desc_size * > ZYNQMP_DMA_NUM_DESCS), > &chan->desc_pool_p, GFP_KERNEL); > > dma_free_coherent(chan->dev,(2 * ZYNQMP_DMA_DESC_SIZE(chan) * > ZYNQMP_DMA_NUM_DESCS), > chan->desc_pool_v, chan->desc_pool_p); > > The arguments desc_size and ZYNQMP_DMA_NUM_DESCS are 32 bit. Though > this overflow condition is not observed but it is a potential problem > in the case of 32-bit multiplication. Hence fix it by using typecast. > > Addresses-Coverity: Event overflow_before_widen. > Signed-off-by: Shravya Kumbham > Reviewed-by: Radhey Shyam Pandey Thanks for the patch. Your SoB is missing in this and the other patches of this series. > --- > drivers/dma/xilinx/zynqmp_dma.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c > index 5fecf5aa6e85..2d0eba25739d 100644 > --- a/drivers/dma/xilinx/zynqmp_dma.c > +++ b/drivers/dma/xilinx/zynqmp_dma.c > @@ -490,7 +490,8 @@ static int zynqmp_dma_alloc_chan_resources(struct dma_chan *dchan) > } > > chan->desc_pool_v = dma_alloc_coherent(chan->dev, > - (2 * chan->desc_size * ZYNQMP_DMA_NUM_DESCS), > + ((size_t)(2 * chan->desc_size) * > + ZYNQMP_DMA_NUM_DESCS), Wouldn't it be easier to change the type of chan->desc_size to size_t? Maybe we could also just calculate the size of the descriptor pool during probe() to make the code more readable? I also noticed that there is the ZYNQMP_DMA_DESC_SIZE() macro, which is inconsistently used in the driver. Maybe you could cleanup this as well as you are at it? > &chan->desc_pool_p, GFP_KERNEL); > if (!chan->desc_pool_v) > return -ENOMEM; > @@ -677,7 +678,8 @@ static void zynqmp_dma_free_chan_resources(struct dma_chan *dchan) > zynqmp_dma_free_descriptors(chan); > spin_unlock_irqrestore(&chan->lock, irqflags); > dma_free_coherent(chan->dev, > - (2 * ZYNQMP_DMA_DESC_SIZE(chan) * ZYNQMP_DMA_NUM_DESCS), > + ((size_t)(2 * ZYNQMP_DMA_DESC_SIZE(chan)) * > + ZYNQMP_DMA_NUM_DESCS), With a pre-calculated descriptor pool size, recalculating the size here wouldn't be necessary anymore. Michael > chan->desc_pool_v, chan->desc_pool_p); > kfree(chan->sw_desc_pool); > pm_runtime_mark_last_busy(chan->dev); > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel