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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B859C433F5 for ; Tue, 5 Oct 2021 08:21:12 +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 E58C761352 for ; Tue, 5 Oct 2021 08:21:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E58C761352 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=foss.st.com 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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:CC:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=QdpbXYaa9Xdykdb3Ze+bMfj+MVIYnOtzJK5vGsjcJ/o=; b=3+ZZP8zUnS/QsjwBXpIbIU1DDj /Z4uSb3A0yld6zqP0YMhncxHIyR28lRsptYsu4IMksUhDdZFQHBVP82iqkMrwEycEOGO82I1QRSW8 En6y9r3+vglo1j6kP0/DDwOYCW/8SfkBLknN1K4dCyy0ZhUIRqAi/TifqnSv7RNKqEa2Zk45Bh5P1 BpagxBCKt9mhw9t63kIz5gOAB8vX5ERcaiObsh8iN29oU0jI7VnNVMB1A/S27W7jyQbMl6W/pqidB JV24UUDo+JahyGT5Or6y87LhEbPmFyvSmp+NY1JQ6rf+n9FHZvL0zD6KIfbAsWrQcBHDCw0xLj0JL Dc0apkaw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mXff6-009Vsc-CU; Tue, 05 Oct 2021 08:18:56 +0000 Received: from mx07-00178001.pphosted.com ([185.132.182.106]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mXff2-009VqU-2B for linux-arm-kernel@lists.infradead.org; Tue, 05 Oct 2021 08:18:53 +0000 Received: from pps.filterd (m0241204.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1952IMrp032013; Tue, 5 Oct 2021 10:18:37 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=rvTXK7UCH3kxOfjWH75rXK7MS8enLLD50wSRBR+Rn4M=; b=2kxAcWWdKJiFZsAdkoe7e75BBNxOq9UfAJSML32VlRP+YVrVM782zAFlR5SuA1DdVBQP 4zd2pQ4a8NmWoP7uL/GMV5z6dzLZtzrvXyXWPcYi1QRWGPjX/17WaVoMWWC1Wey66Hy2 FCUXWhCg8P0clVPx8UZ66kD3pKJ+3D/TWyam9x3kVmCADLRqqU+W5CPMwUi0dknSJeCr UZPiMUj6pNjQ8zLH18Hx42ZooVF3KxRifqseT1SWvPLzQATuSZJa5oNLFx3HuPdAOTro DmIXdSXRtcHdvcDaVG+92betXVgYmwZyHElb42w4UxEsaj4hgfu1AdQUHgsETv2VgB6N +A== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com with ESMTP id 3bgdt9sjtw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 05 Oct 2021 10:18:37 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 0B2B110002A; Tue, 5 Oct 2021 10:18:36 +0200 (CEST) Received: from Webmail-eu.st.com (sfhdag2node2.st.com [10.75.127.5]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 018E9222C86; Tue, 5 Oct 2021 10:18:36 +0200 (CEST) Received: from lmecxl0577.lme.st.com (10.75.127.48) by SFHDAG2NODE2.st.com (10.75.127.5) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Tue, 5 Oct 2021 10:18:34 +0200 Subject: Re: [PATCH] iio: adc: stm32-dfsdm: Fix the uninitialized use if regmap_read() fails To: Jonathan Cameron , Yizhuo , "Mugilraj Dhavachelvan" , Olivier Moysan , Krzysztof Kozlowski , Arnaud Pouliquen , CC: Lars-Peter Clausen , Maxime Coquelin , Alexandre Torgue , Andy Shevchenko , Mark Brown , , , References: <20210719195313.40341-1-yzhai003@ucr.edu> <20210724164840.7381053b@jic23-huawei> <20210808183243.70619aa8@jic23-huawei> <20211003164726.42e20526@jic23-huawei> From: Olivier MOYSAN Message-ID: <9d8d6633-deba-bcf8-f717-68def3ef798e@foss.st.com> Date: Tue, 5 Oct 2021 10:18:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20211003164726.42e20526@jic23-huawei> Content-Language: en-US X-Originating-IP: [10.75.127.48] X-ClientProxiedBy: SFHDAG2NODE1.st.com (10.75.127.4) To SFHDAG2NODE2.st.com (10.75.127.5) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.391,FMLib:17.0.607.475 definitions=2021-10-04_05,2021-10-04_01,2020-04-07_01 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211005_011852_547360_BB06FBD1 X-CRM114-Status: GOOD ( 30.42 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On 10/3/21 5:47 PM, Jonathan Cameron wrote: > On Sun, 8 Aug 2021 18:32:43 +0100 > Jonathan Cameron wrote: > >> On Sat, 24 Jul 2021 16:48:40 +0100 >> Jonathan Cameron wrote: >> >>> On Mon, 19 Jul 2021 19:53:11 +0000 >>> Yizhuo wrote: >>> >>>> Inside function stm32_dfsdm_irq(), the variable "status", "int_en" >>>> could be uninitialized if the regmap_read() fails and returns an error >>>> code. However, they are directly used in the later context to decide >>>> the control flow, which is potentially unsafe. >>>> >>>> Fixes: e2e6771c64625 ("IIO: ADC: add STM32 DFSDM sigma delta ADC support") >>>> >>>> Signed-off-by: Yizhuo >>> >>> Hi Yizhou >>> >>> I want to get some review of this from people familiar with the >>> hardware as there is a small possibility your reordering might have >>> introduced a problem. >> >> To stm32 people, can someone take a look at this? > > This one is still outstanding. If anyone from stm32 side of things could take a look > that would be great, > > Jonathan > I cannot see side effects with reordering itself. However, if we get an error with the read access, just leaving with irq_handled status is probably not enough. In such case we are facing a serious issue and it would make sense to return irq_none instead, as the interrupt will probably never be acknowledged. BRs >> >> Thanks, >> >> Jonathan >> >>> >>>> --- >>>> drivers/iio/adc/stm32-dfsdm-adc.c | 9 +++++++-- >>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c >>>> index 1cfefb3b5e56..d8b78aead942 100644 >>>> --- a/drivers/iio/adc/stm32-dfsdm-adc.c >>>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c >>>> @@ -1292,9 +1292,11 @@ static irqreturn_t stm32_dfsdm_irq(int irq, void *arg) >>>> struct stm32_dfsdm_adc *adc = iio_priv(indio_dev); >>>> struct regmap *regmap = adc->dfsdm->regmap; >>>> unsigned int status, int_en; >>>> + int ret; >>>> >>>> - regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status); >>>> - regmap_read(regmap, DFSDM_CR2(adc->fl_id), &int_en); >>> >>> Moving this later is only valid if there aren't any side effects. >>> The current ordering is strange enough it makes me wonder if there might be! >>> >>> Jonathan >>> >>>> + ret = regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status); >>>> + if (ret) >>>> + return IRQ_HANDLED; >>>> >>>> if (status & DFSDM_ISR_REOCF_MASK) { >>>> /* Read the data register clean the IRQ status */ >>>> @@ -1303,6 +1305,9 @@ static irqreturn_t stm32_dfsdm_irq(int irq, void *arg) >>>> } >>>> >>>> if (status & DFSDM_ISR_ROVRF_MASK) { >>>> + ret = regmap_read(regmap, DFSDM_CR2(adc->fl_id), &int_en); >>>> + if (ret) >>>> + return IRQ_HANDLED; >>>> if (int_en & DFSDM_CR2_ROVRIE_MASK) >>>> dev_warn(&indio_dev->dev, "Overrun detected\n"); >>>> regmap_update_bits(regmap, DFSDM_ICR(adc->fl_id), >>> >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel