From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (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 489972F7455 for ; Fri, 21 Nov 2025 09:50:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763718610; cv=none; b=RBK70M5CbJfKHN0KtPya/ZWQ3AXJzXLo4sGWyCmi7qxP3OkyjvcKJ/0GW+VtWNs/IkehQ/1fCa319fqjqHkzYIbjYF1VbPPIjF6J64tSTFVV9FCyyhl75uj1pkYWH4Iql3ctzeofKni9YLt35FiGkwBExf8lX+oU7fmPIL92zg0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763718610; c=relaxed/simple; bh=5RQ1tRswWgLCT5SZfLOjNXkN5DTiCQ9C+sKFidpyF3E=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Ivm3EwubwBOwJq+TJG3QuFtqWILTSemkSDIezDn0Wi2rbWP9t0CfZnbhQo11szoKefjTNEJhIykqe2tiyf+CBMCAXS1COwjg9lmKP6aPhrGN7ZRgG4fwaGcvrz72GYNUKf9yZpZaYkAZmEbEBBv89nFWEpYxRWZwpM54f3K0m7w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=KjszPjLA; arc=none smtp.client-ip=209.85.128.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="KjszPjLA" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-477bf34f5f5so5061705e9.0 for ; Fri, 21 Nov 2025 01:50:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1763718605; x=1764323405; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=dDUGi6ZGHsutZYAEKUijNwAxRhV3XAznuzlZLBKkNcA=; b=KjszPjLACXlsix3VeBuxq4KJrYk5zk1zlmlDCGOz2juT/QtNFgNOY1nw0S4uwzSJfc dHTtFXOPhYxUTPHid+3FzFIj9HJ1jajpNwrttcFtznaStAzonYm79kI7FARy2/sxXBuk 146Gl9Vi5bzE8ZyBOXpo3q+d4i7XXAiTtRtUyD6b23MJvu3xE/I76Eh1XxLC67HI0UcS RRGiGVBxGkFjMx0T8kN/1rJGyGpC2akmi1hDtfzAXGNGEcKQQSYQkEiGrJ4NRbJBKrGp xjL0gsuMd9PcVXzRiZ8/K0LM/B8b2e41OS8WJ2ZiIWSB2guH4SDFjcujhRcxyNJpaz7g +FDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763718605; x=1764323405; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=dDUGi6ZGHsutZYAEKUijNwAxRhV3XAznuzlZLBKkNcA=; b=BUjvFGVlKdE+4axxkIwLiT97k6b2kcNr1KtEp1GBqc75IX1Ec89rqIbSDqXDrDd+ps KzgKFlUV/5NSdPdrilBTCu8k6wMv05KydtWk8uMq91enT16RGvRaAJm7w38ILGsXl2z8 ifVYXMserDzFxmiAO+XQ/6rt6AA1jNQSEidS5kkgL0swM6sZ0WkwxH2FNdImA3em+d+x 1Npb1jXVt1KCy0HLgPSaz9gUeCOmcqxcb6yIFYffPiGybtjI/GYmt6tqBfuWSIqoUZj5 vv9N3UWBjMZXeiQ86YTNbCt43EavCj0URR8OF61HfmkDsZkmWUdp/+nxDVSiVXf9+PBp bbyg== X-Forwarded-Encrypted: i=1; AJvYcCVizwGz/eH1Z3AHU0+yEvrOGrNEqsZjcWZX/LHrbF4/i/mrokPm3/7Jq68X6WC21pwC0Ns00T4gIGvEPZs=@vger.kernel.org X-Gm-Message-State: AOJu0YyVbJVdCF6UqcIlzMAFV3X5JGmYmIEI6FDQ1BisNB4/NVOCAOIV 5efEjvHtYFI3ybAmKg+JGDzAsm3u3UxIqeGALncKlZPGZNxmaUW5heEFXWYitet+ntA= X-Gm-Gg: ASbGncvlIK2CQRiZj1HIB47/TqGoEjxi3hEVPeCglswg/nIwQjNjU+uCE3KBoF4oyu3 /N9AsxU+qR7HHl/MfOZe25Fa5cj4fJFtKmJG/FKKDuYqo4RyJ+Wp2egf3ly8HmL+bI4IRTnSMuQ jFvn63iSGqyIt+m6s4Nxvh90hK2TTg9+ZmemssajN1l319B0YxRMiJ60fZY+V2NiFhu7VBdqjVq LByFOY+zDM5Jlm3pD5/pFaFFdfPtFj2ae0fM44pteVv1RX/el8yD/6YieoGF+/bB7rSIgZGb57p QTdtml6cN+wPl5W2rgaNcp/PrXcJxtkS0uVs2wdpWerw13hS92kVOyQ6dPgYK1thQR5MPLC0ten 3sELpnFyjygzCGUBmC73tVWsZs8usWXyp3kTrAL42sV4x9jHWRBLPdMAHpRmGOW7MyzjePIroNp CESNe9W8MjUnbmx1MWKvvJF6KnZLU= X-Google-Smtp-Source: AGHT+IE8M+EHxlJLPo2q1ptcszz4xTi5Hcpr4PPexE02LnvTImirC4/2Tss40XTDEV8NnTZCE45Gbw== X-Received: by 2002:a5d:5f48:0:b0:42b:32f5:ad18 with SMTP id ffacd0b85a97d-42cc1cd5deamr1732407f8f.9.1763718605414; Fri, 21 Nov 2025 01:50:05 -0800 (PST) Received: from [192.168.1.3] ([185.48.77.170]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42cb7f34fddsm10479833f8f.14.2025.11.21.01.50.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Nov 2025 01:50:05 -0800 (PST) Message-ID: <172ca2d9-4a6f-4498-bdfd-8aa7428581ce@linaro.org> Date: Fri, 21 Nov 2025 09:50:03 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] coresight: etm3x: Fix buffer overwrite in cntr_val_show() To: Kuan-Wei Chiu , suzuki.poulose@arm.com Cc: 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 References: <20251121002350.1166758-1-visitorckw@gmail.com> Content-Language: en-US From: James Clark In-Reply-To: <20251121002350.1166758-1-visitorckw@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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. 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;