From patchwork Wed Jul 1 16:08:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Poirier X-Patchwork-Id: 11636799 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5842160D for ; Wed, 1 Jul 2020 16:10:27 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 2F52020809 for ; Wed, 1 Jul 2020 16:10:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="2zpTvCKg"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="MWx3LtVl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2F52020809 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-Id:Date: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Qhdu2izX31C5DiNU+x2K0xZaYXHZsyNcUDq1WPd1Lg0=; b=2zpTvCKgV62pAcXgQ758hGqdy GlLwKeP7Kg2+gOhx2EoT+5aERrGUQHBNOuLNaRdEVPRbtEa9WjbuKgCej6nZd+67fnHTayTnPVI3m kl2yRhDiBzmF20YiymGX7t8NFx5/b6czXDZzgfvDW/QWB89NpNujvQnbZoic5CCIy3e8glv9az64K rwjlVyJCZaW8OVILVJKY7etiNMjAaYr7DQruPuqjlEV6NPdUWFACVO143ZZrQoZGI654hdIFrISxv Sj3yu8V3wQG5PR4R/vv1yIXVlIetrcwhg9oEuZ4JqvC1LR2FAJueim3wuzBofJxJkrTAsFvJrLIdk XHM+UVkzA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqfII-0002Fz-0r; Wed, 01 Jul 2020 16:09:06 +0000 Received: from mail-pl1-x641.google.com ([2607:f8b0:4864:20::641]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqfIA-0002CV-OU for linux-arm-kernel@lists.infradead.org; Wed, 01 Jul 2020 16:08:59 +0000 Received: by mail-pl1-x641.google.com with SMTP id x8so9196199plm.10 for ; Wed, 01 Jul 2020 09:08:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=50q1h2Wye3LWWhGs+r/wMnFXXARndfXMfBC/UWtScHU=; b=MWx3LtVl+uTXcFGfP31J8sbrQO9ojK7lBg7oT1JSwLqUBCR8Sw2HpBAuY44Qwss605 ZHQEQXYGrxBVWkSlx/z9blbryWOHJ2guKXBbFWtW0kVARPrXIi7/LSVKK6I5e2dfXYz8 7yZWI/S1T5mIMtFvIbLTZm3q8R3qWR/s+I/6n+D0IU+wnGziBrnqz5yNkkHgbfqgcKAT 184s/llu9qdAoKftjM1UEsAxcXpCikPZffozKffR3RAFGFPd4dp4CWbz0Q3ASjT/ogJd ur2+yKg29LzAgbP/6+Bqba/dNYBw8UIeVI+8ALkaa/50lRDAsZu3qYgvdn4Ykil/Wh6v xRiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=50q1h2Wye3LWWhGs+r/wMnFXXARndfXMfBC/UWtScHU=; b=EOyel+0H3cB1FaFVvVJn0XRtnxk6PdOEfN+FTMvxwia+4onz7Zp1mwNHAHC4dCHBhT H9iLsk+ZgvObwbOC56JcdrxL+FvDqQRmEgyGUnwR7tMd12HBdkJD/xuYgbi4YSoQsChq eOJy0ZXsUigh+6BWIvjnSm6pdfKsvlCleGH2Mo0vQsgzBfdQuPyqkT82Q+6PsJBC5pRp gkrNnjT+hn6SMP2BKb5xdf0WFNRUfFLo+d487cNF89Huh5uWl+PWDwArA5PkEGOhBE0Q JdH7Rr6Iq0PSSpoJ/dq/YIfk6nrYNojP0cc8BauqZWr+HLK4183dbfXfCd/o6M36sRna 8KzQ== X-Gm-Message-State: AOAM5304tuQl5cx07WVai7KWUCfNMeOY9DnWJokEmkVF5QT+fbk0z2bx A8+Z/vqUM9l7nWqjQoUOvzFumPqR5lk= X-Google-Smtp-Source: ABdhPJzcWbV+Y5LsTDEBDByGP9UF0P2oOxQhmUzZG4h2xmG9+Mc4T1dlPIe2QRF+J+GBcTQLhQrgzQ== X-Received: by 2002:a17:90a:ea86:: with SMTP id h6mr27231013pjz.167.1593619736744; Wed, 01 Jul 2020 09:08:56 -0700 (PDT) Received: from xps15.cg.shawcable.net (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id b82sm5123327pfb.215.2020.07.01.09.08.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Jul 2020 09:08:54 -0700 (PDT) From: Mathieu Poirier To: gregkh@linuxfoundation.org Subject: [PATCH 1/2] coresight: cti: Fix error handling in probe Date: Wed, 1 Jul 2020 10:08:51 -0600 Message-Id: <20200701160852.2782823-2-mathieu.poirier@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200701160852.2782823-1-mathieu.poirier@linaro.org> References: <20200701160852.2782823-1-mathieu.poirier@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200701_120858_974371_854FCB9B X-CRM114-Status: GOOD ( 20.53 ) X-Spam-Score: -0.2 (/) X-Spam-Report: SpamAssassin version 3.4.4 on merlin.infradead.org summary: Content analysis details: (-0.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2607:f8b0:4864:20:0:0:0:641 listed in] [list.dnswl.org] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-kernel@lists.infradead.org, dan.carpenter@oracle.com, mike.leach@linaro.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org From: Dan Carpenter There were a couple problems with error handling in the probe function: 1) If the "drvdata" allocation failed then it lead to a NULL dereference. 2) On several error paths we decremented "nr_cti_cpu" before it was incremented which lead to a reference counting bug. There were also some parts of the error handling which were not bugs but were messy. The error handling was confusing to read. It printed some unnecessary error messages. The simplest way to fix these problems was to create a cti_pm_setup() function that did all the power management setup in one go. That way when we call cti_pm_release() we don't have to deal with the complications of a partially configured power management config. I reversed the "if (drvdata->ctidev.cpu >= 0)" condition in cti_pm_release() so that it mirros the new cti_pm_setup() function. Fixes: 6a0953ce7de9 ("coresight: cti: Add CPU idle pm notifer to CTI devices") Signed-off-by: Dan Carpenter Reviewed-by: Mike Leach Reviewed-by: Mathieu Poirier --- drivers/hwtracing/coresight/coresight-cti.c | 96 ++++++++++++--------- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-cti.c b/drivers/hwtracing/coresight/coresight-cti.c index 40387d58c8e7..3ccc703dc940 100644 --- a/drivers/hwtracing/coresight/coresight-cti.c +++ b/drivers/hwtracing/coresight/coresight-cti.c @@ -747,17 +747,50 @@ static int cti_dying_cpu(unsigned int cpu) return 0; } +static int cti_pm_setup(struct cti_drvdata *drvdata) +{ + int ret; + + if (drvdata->ctidev.cpu == -1) + return 0; + + if (nr_cti_cpu) + goto done; + + cpus_read_lock(); + ret = cpuhp_setup_state_nocalls_cpuslocked( + CPUHP_AP_ARM_CORESIGHT_CTI_STARTING, + "arm/coresight_cti:starting", + cti_starting_cpu, cti_dying_cpu); + if (ret) { + cpus_read_unlock(); + return ret; + } + + ret = cpu_pm_register_notifier(&cti_cpu_pm_nb); + cpus_read_unlock(); + if (ret) { + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING); + return ret; + } + +done: + nr_cti_cpu++; + cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata; + + return 0; +} + /* release PM registrations */ static void cti_pm_release(struct cti_drvdata *drvdata) { - if (drvdata->ctidev.cpu >= 0) { - if (--nr_cti_cpu == 0) { - cpu_pm_unregister_notifier(&cti_cpu_pm_nb); + if (drvdata->ctidev.cpu == -1) + return; - cpuhp_remove_state_nocalls( - CPUHP_AP_ARM_CORESIGHT_CTI_STARTING); - } - cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL; + cti_cpu_drvdata[drvdata->ctidev.cpu] = NULL; + if (--nr_cti_cpu == 0) { + cpu_pm_unregister_notifier(&cti_cpu_pm_nb); + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_CTI_STARTING); } } @@ -823,19 +856,14 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) /* driver data*/ drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); - if (!drvdata) { - ret = -ENOMEM; - dev_info(dev, "%s, mem err\n", __func__); - goto err_out; - } + if (!drvdata) + return -ENOMEM; /* Validity for the resource is already checked by the AMBA core */ base = devm_ioremap_resource(dev, res); - if (IS_ERR(base)) { - ret = PTR_ERR(base); - dev_err(dev, "%s, remap err\n", __func__); - goto err_out; - } + if (IS_ERR(base)) + return PTR_ERR(base); + drvdata->base = base; dev_set_drvdata(dev, drvdata); @@ -854,8 +882,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) pdata = coresight_cti_get_platform_data(dev); if (IS_ERR(pdata)) { dev_err(dev, "coresight_cti_get_platform_data err\n"); - ret = PTR_ERR(pdata); - goto err_out; + return PTR_ERR(pdata); } /* default to powered - could change on PM notifications */ @@ -867,35 +894,20 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) drvdata->ctidev.cpu); else cti_desc.name = coresight_alloc_device_name(&cti_sys_devs, dev); - if (!cti_desc.name) { - ret = -ENOMEM; - goto err_out; - } + if (!cti_desc.name) + return -ENOMEM; /* setup CPU power management handling for CPU bound CTI devices. */ - if (drvdata->ctidev.cpu >= 0) { - cti_cpu_drvdata[drvdata->ctidev.cpu] = drvdata; - if (!nr_cti_cpu++) { - cpus_read_lock(); - ret = cpuhp_setup_state_nocalls_cpuslocked( - CPUHP_AP_ARM_CORESIGHT_CTI_STARTING, - "arm/coresight_cti:starting", - cti_starting_cpu, cti_dying_cpu); - - if (!ret) - ret = cpu_pm_register_notifier(&cti_cpu_pm_nb); - cpus_read_unlock(); - if (ret) - goto err_out; - } - } + ret = cti_pm_setup(drvdata); + if (ret) + return ret; /* create dynamic attributes for connections */ ret = cti_create_cons_sysfs(dev, drvdata); if (ret) { dev_err(dev, "%s: create dynamic sysfs entries failed\n", cti_desc.name); - goto err_out; + goto pm_release; } /* set up coresight component description */ @@ -908,7 +920,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) drvdata->csdev = coresight_register(&cti_desc); if (IS_ERR(drvdata->csdev)) { ret = PTR_ERR(drvdata->csdev); - goto err_out; + goto pm_release; } /* add to list of CTI devices */ @@ -927,7 +939,7 @@ static int cti_probe(struct amba_device *adev, const struct amba_id *id) dev_info(&drvdata->csdev->dev, "CTI initialized\n"); return 0; -err_out: +pm_release: cti_pm_release(drvdata); return ret; }