From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7624736D4E6 for ; Thu, 27 Nov 2025 09:22:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764235356; cv=none; b=IMJ533qgHFNAjG3Kp6LyMtknUi4o4lryKOMS8I6ftqrODDNJBcjL0aB6RCRpawO5pcpEEyzPeyLFK1FedNn3UfiMR9zsE3MGi9VsA3EqivlnAfOoum6RAgouXAUQ/yYYHerfXsu6t1Bkez1oAlTKltdrkSPzYhCSGji5K8nr6Y0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764235356; c=relaxed/simple; bh=3KXw504Qjuh3YNTPtDwCRZtvq0M4mDJzoNrU3v52CsU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lSX2h6Nr3xmi753iPbhrqBmd4mZR6ZtIunJMkPNrVfB1tZ5y0fNg3ZsUCs0ZWqRzK6ojRK5V58BatYH4V/0QK+jEmbfnCDnPdJpt0HL4l6WHxcty+s2arTSJKbvk13+EPIRxw9KEOclKmnRyvvQl15Yi/om9mtrxvCDF84Txji0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 55FD91570; Thu, 27 Nov 2025 01:22:26 -0800 (PST) Received: from localhost (e132581.arm.com [10.1.196.87]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 61E463F66E; Thu, 27 Nov 2025 01:22:33 -0800 (PST) Date: Thu, 27 Nov 2025 09:22:31 +0000 From: Leo Yan To: Kuan-Wei Chiu Cc: James Clark , Mike Leach , suzuki.poulose@arm.com, alexander.shishkin@linux.intel.com, pratikp@codeaurora.org, mathieu.poirier@linaro.org, gregkh@linuxfoundation.org, jserv@ccns.ncku.edu.tw, marscheng@google.com, ericchancf@google.com, milesjiang@google.com, nickpan@google.com, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] coresight: etm3x: Fix buffer overwrite in cntr_val_show() Message-ID: <20251127092231.GN724103@e132581.arm.com> References: <20251121002350.1166758-1-visitorckw@gmail.com> <172ca2d9-4a6f-4498-bdfd-8aa7428581ce@linaro.org> <05babb6d-a588-49f8-a34a-c82d5f58adf5@linaro.org> <68efd5a2-3b65-4f37-9bf0-40c4e5ade480@linaro.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Thu, Nov 27, 2025 at 04:44:53PM +0800, Kuan-Wei Chiu wrote: [...] > I don't feel it is my place to say whether the etm3x driver should be > removed entirely. Sorry for confusion. Your fix patch is welcome, this is useful no matter if remove the ETMv3 driver or not. > However, if we decide to keep it, I agree that aligning cntr_val_show > with the cntr_val_store behavior (using cntr_idx) makes more sense. > > Here is my plan for v2: > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > index 762109307b86..77578885e8f3 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > @@ -717,26 +717,19 @@ static DEVICE_ATTR_RW(cntr_rld_event); > static ssize_t cntr_val_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - int i, ret = 0; > u32 val; > struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent); > struct etm_config *config = &drvdata->config; > > if (!coresight_get_mode(drvdata->csdev)) { > spin_lock(&drvdata->spinlock); > - for (i = 0; i < drvdata->nr_cntr; i++) > - ret += sprintf(buf, "counter %d: %x\n", > - i, config->cntr_val[i]); > + val = config->cntr_val[config->cntr_idx]; > spin_unlock(&drvdata->spinlock); > - return ret; > - } > - > - for (i = 0; i < drvdata->nr_cntr; i++) { > - val = etm_readl(drvdata, ETMCNTVRn(i)); > - ret += sprintf(buf, "counter %d: %x\n", i, val); > + return sprintf(buf, "%x\n", val); > } > > - return ret; > + val = etm_readl(drvdata, ETMCNTVRn(config->cntr_idx)); It is not right to read register at here (it cannot promise to read the CPU (cp14) register on the target CPU). Please refer to the same function in coresight-etm4x-sysfs.c. I think we can do the same thing at here. > + return sprintf(buf, "%x\n", val); > } > > static ssize_t cntr_val_store(struct device *dev, > > > Given the upcoming merge window, I plan to submit this v2 after -rc1 > is released. > > Alternatively, if the consensus is to drop the driver, I am happy to > submit a patch for that instead. Please continue this patch. Thanks a lot! Leo