From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B216D2EB875 for ; Fri, 21 Nov 2025 17:02:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763744560; cv=none; b=qXaAK9WOh7aHfDIcCqUIZNAR/C7+t34H6IN7Kr/ZovchQ+e64H4xuxVCraKYCB3Tald3gMlTq+M1WxFujtTGli8/XMwWks3YQ45zcQ073KlZPDboYIEuNvSY+TKfX4sN8KSV5SChTMcd6+u30hhihc50igCcONbFcmVdYL48AoQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763744560; c=relaxed/simple; bh=pUJgnTRR2htQ0KsWDwdZuGmw73ZYAPHiVSQ/XyhwYKo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TxMWmzivFUPAi0xW00DxgpsFgsg8fMXqyE4UiVCydyctKPVPUTkI/laZY7QFySahDxsJC+VHuVycptyNeg6TXuZQgXKrK3+3V7Oqr/f4ozyYOjuPpnp8xfZe6I7SUCBJvR2g92XSScgTYyxBZjOtNZv1/BFZIiuuMtViNQrbcKw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=aekxI5r7; arc=none smtp.client-ip=209.85.214.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="aekxI5r7" Received: by mail-pl1-f177.google.com with SMTP id d9443c01a7336-2955623e6faso27379475ad.1 for ; Fri, 21 Nov 2025 09:02:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763744558; x=1764349358; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=lymtWkABTw0hdE/FIgSwpoWnmefZZJJLODll+UbmvVc=; b=aekxI5r7JT5wrGrgkY9EuOV0OkNrgZs1HhNQwidPOOJLPIHjHy5bv3OXnISlZsbOuv MRuQbuKFCynVyi2PIZP2NzSE2Vix3LEcchQQguyNIZrQfHS+5CMfTYJJfiW5zeOSrv3y Ct7tLYDfSCmaVjXNuxr0DF9kI1InUtAIWsteXD27chPZMyB3FAk8xbXhOWq9xxeo+oWf uQcjZGvY/Lb8RmH3dU0RGFu7HnYPCnFZay3YH5/HpIck8+mjUdBfPMENLAl+lQ9cYNdj iDZGzMS9JXi0RFbZctnTr/zapKnmtzO6qfKC6TMaKGItExOz4Vddwxm82YkBpICUr9Bs ViqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763744558; x=1764349358; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lymtWkABTw0hdE/FIgSwpoWnmefZZJJLODll+UbmvVc=; b=CHlaT4cM3n+uxv+zEdi3NCgoEzjLthcQFF74PX8Ldcts+D0Yon9Q2AVkr2M6TjEDxa Gpzv0Ypgyn7zdSMHZNLb0LTpF6dAEXcKrxH0LetlRZs6UDbBPmPWZM6khDKwyK+autPS Y1G2CoL+PnEQ6yvc7xmlA3vHLA0bJisVH3GU3xzXfqRYOt7v6rwJN/3dGRljTR5kDxaL 5oFvcqbAjf/evwljz5zE/lX3OY+ERLaL6I2GV12vFWqTzJWGhWifFO2OefDy1W+8ZrpM Cdhl9xWg1M9S/ah09VDaATg/sIMN2bqthGRIaeEWVYRqrn2UvYIrzRXvrqj1rfu5qQCl lFEg== X-Forwarded-Encrypted: i=1; AJvYcCVBIlSkj/J3JPSF8xg207B92bOcNYpz1z1XawEhoTUL6XXseEdrlK0fx0wNsMlSklxT+g+qa6Bi6PI9iNM=@vger.kernel.org X-Gm-Message-State: AOJu0YxULL03dlg+nenxDtbaTS807MbXmQOeBxvvbDGJbOj9L4W1M60J f3Pa5bgp0bv0wc3DmZA2ETdkwOwdqAB6meQAslwB6IMHMuyrLUuamoZ6ffDoePXV X-Gm-Gg: ASbGnctVF3FXsg+Ge0L1AcN9nAce4FXRTsMZe5CaMIYDi7/AenCdgi3h9Ti/QDmS0cb nD6fiPhQSVuHQGXDRGhMUUB/2DwPrgJLINJMXZmKJAeQLQRXRXlJqzWXVQwDMZouyn8i99IabI1 uqJMqXEwg/rBplp2WWP9GoQlBG2n1gaXPUz2PF18w8SGZOa9aiYHp8zt2T3L1ih7lqHQt6RVqTD lEx1X3XIeRRveh/SBjtFYbIjnhZqCOQ8YlSjfUuAbDpBAjPUzdijWOv71uL/t/a2R3czTqyWn0o XpCHw5N7E1trPbEVgq9nWS41YGqywoi1BsDI3gBtkzR8eAqitJSiIBnC+cY7AsR/9EgTfl04MWg gvvx6iTbz+4ZeJxpE0EMaFHgAdDHIc+OPgRnoGYhqrySv0X45HDgvBjVJbR87xTh5a+wtJvS2wB P8boizP4GP1kebdqP7nBFPAYPB X-Google-Smtp-Source: AGHT+IESX0iIZC54W5N9ogWNgFKNFvkXrfEsHoQEyi+cB9v/cq+ARm6HldAqJQP+IzueGtEYdLGiog== X-Received: by 2002:a17:903:384c:b0:27c:56af:88ea with SMTP id d9443c01a7336-29b6bfa1f66mr38248585ad.60.1763744557293; Fri, 21 Nov 2025 09:02:37 -0800 (PST) Received: from google.com ([2402:7500:499:ceaa:ae86:a8ee:bdfa:37bc]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-29b5b13e7e6sm61238345ad.40.2025.11.21.09.02.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 21 Nov 2025 09:02:36 -0800 (PST) Date: Sat, 22 Nov 2025 01:02:29 +0800 From: Kuan-Wei Chiu To: James Clark Cc: suzuki.poulose@arm.com, mike.leach@linaro.org, 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: References: <20251121002350.1166758-1-visitorckw@gmail.com> <172ca2d9-4a6f-4498-bdfd-8aa7428581ce@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: <172ca2d9-4a6f-4498-bdfd-8aa7428581ce@linaro.org> Hi James, On Fri, Nov 21, 2025 at 09:50:03AM +0000, James Clark wrote: > > > On 21/11/2025 12:23 am, Kuan-Wei Chiu wrote: > > The cntr_val_show() function is meant to display the values of all > > available counters. However, the sprintf() call inside the loop was > > always writing to the beginning of the buffer, causing the output of > > previous iterations to be overwritten. As a result, only the value of > > the last counter was actually returned to the user. > > > > Fix this by using the return value of sprintf() to calculate the > > correct offset into the buffer for the next write, ensuring that all > > counter values are appended sequentially. > > > > Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver") > > Signed-off-by: Kuan-Wei Chiu > > --- > > Build tested only. I do not have the hardware to run the etm3x driver, > > so I would be grateful if someone could verify this on actual hardware. > > > > I noticed this issue while browsing the coresight code after attending > > a technical talk on the subject. This code dates back to the initial > > driver submission over 10 years ago, so I was surprised it hadn't been > > caught earlier. Although I cannot perform runtime testing, the logic > > error seems obvious to me, so I still decided to submit this patch. > > Nice find. I think the point that it wasn't caught changes how we fix it. > Either nobody used it ever - so we can just delete it. Or someone was using > it and they expect it to always return a single entry with the value of the > last counter and this is a potentially breaking change. So maybe instead of > fixing this we should add a new cntr_vals_show() which works correctly. But > then again if nobody is using it we shouldn't do that either. Thanks for your feedback. I agree that if any tool relies on the current behavior, this patch would break the ABI and violate the hard rule that we must never break userspace. However, I am not sure how to determine if any userspace tools are actually using this sysfs interface. Is there a recommended way to verify this, or a standard procedure/convention to follow in this situation? Regards, Kuan-Wei > > The interface isn't even that great, it should be a separate file per > counter. You don't want to be parsing strings and colons to try to read a > single value, especially in C. Separate files allows you to read it directly > without any hassle. > > > > > drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > > index 762109307b86..312033e74b7a 100644 > > --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > > +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c > > @@ -725,7 +725,7 @@ static ssize_t cntr_val_show(struct device *dev, > > 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", > > + ret += sprintf(buf + ret, "counter %d: %x\n", > > i, config->cntr_val[i]); > > spin_unlock(&drvdata->spinlock); > > return ret; > > @@ -733,7 +733,7 @@ static ssize_t cntr_val_show(struct device *dev, > > for (i = 0; i < drvdata->nr_cntr; i++) { > > val = etm_readl(drvdata, ETMCNTVRn(i)); > > - ret += sprintf(buf, "counter %d: %x\n", i, val); > > + ret += sprintf(buf + ret, "counter %d: %x\n", i, val); > > } > > return ret; >