Received-SPF: pass (google.com: domain of linux-kernel+bounces-2193-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
To: Tejun Heo <tj@kernel.org>,
	Josef Bacik <josef@toxicpanda.com>,
	Jens Axboe <axboe@kernel.dk>,
	Shaohua Li <shli@fb.com>
Cc: linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
	cgroups@vger.kernel.org,
	linux-block@vger.kernel.org
Subject: [PATCH] blk-throttle: Fix some potential string truncation in tg_prfill_limit()
Date: Sat, 16 Dec 2023 13:54:56 +0100
Message-Id: <0461f1d69c84cf5a98ae57012856dace757d319e.1702731206.git.christophe.jaillet@wanadoo.fr>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When compiled with W=1, we get:
  block/blk-throttle.c: In function ‘tg_prfill_limit’:
  block/blk-throttle.c:1539:74: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
   1539 |                         snprintf(idle_time, sizeof(idle_time), " idle=%lu",
        |                                                                          ^
  block/blk-throttle.c:1539:25: note: ‘snprintf’ output between 8 and 27 bytes into a destination of size 26
   1539 |                         snprintf(idle_time, sizeof(idle_time), " idle=%lu",
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1540 |                                 tg->idletime_threshold_conf);
        |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  block/blk-throttle.c:1546:43: error: ‘%lu’ directive output may be truncated writing between 1 and 20 bytes into a region of size 17 [-Werror=format-truncation=]
   1546 |                                 " latency=%lu", tg->latency_target_conf);
        |                                           ^~~
  block/blk-throttle.c:1546:33: note: directive argument in the range [0, 18446744073709551614]
   1546 |                                 " latency=%lu", tg->latency_target_conf);
        |                                 ^~~~~~~~~~~~~~
  block/blk-throttle.c:1545:25: note: ‘snprintf’ output between 11 and 30 bytes into a destination of size 26
   1545 |                         snprintf(latency_time, sizeof(latency_time),
        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1546 |                                 " latency=%lu", tg->latency_target_conf);
        |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In order to fix it, remove all the intermediate buffers and write directly
into the 'sf' seq_file.

Fixes: ada75b6e5b2a ("blk-throttle: add interface to configure idle time threshold")
Fixes: ec80991d6fc2 ("blk-throttle: add interface for per-cgroup target latency")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 block/blk-throttle.c | 52 +++++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 16f5766620a4..470a8a4ed68e 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1494,11 +1494,8 @@ static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd,
 {
 	struct throtl_grp *tg = pd_to_tg(pd);
 	const char *dname = blkg_dev_name(pd->blkg);
-	char bufs[4][21] = { "max", "max", "max", "max" };
 	u64 bps_dft;
 	unsigned int iops_dft;
-	char idle_time[26] = "";
-	char latency_time[26] = "";
 
 	if (!dname)
 		return 0;
@@ -1520,35 +1517,46 @@ static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd,
 	      tg->latency_target_conf == DFL_LATENCY_TARGET)))
 		return 0;
 
+	seq_printf(sf, "%s", dname);
+
+	seq_puts(sf, " rbps=");
 	if (tg->bps_conf[READ][off] != U64_MAX)
-		snprintf(bufs[0], sizeof(bufs[0]), "%llu",
-			tg->bps_conf[READ][off]);
+		seq_printf(sf, "%llu", tg->bps_conf[READ][off]);
+	else
+		seq_puts(sf, "max");
+
+	seq_puts(sf, " wbps=");
 	if (tg->bps_conf[WRITE][off] != U64_MAX)
-		snprintf(bufs[1], sizeof(bufs[1]), "%llu",
-			tg->bps_conf[WRITE][off]);
+		seq_printf(sf, "%llu", tg->bps_conf[WRITE][off]);
+	else
+		seq_puts(sf, "max");
+
+	seq_puts(sf, " riops=");
 	if (tg->iops_conf[READ][off] != UINT_MAX)
-		snprintf(bufs[2], sizeof(bufs[2]), "%u",
-			tg->iops_conf[READ][off]);
+		seq_printf(sf, "%u", tg->iops_conf[READ][off]);
+	else
+		seq_puts(sf, "max");
+
+	seq_puts(sf, " wiops=");
 	if (tg->iops_conf[WRITE][off] != UINT_MAX)
-		snprintf(bufs[3], sizeof(bufs[3]), "%u",
-			tg->iops_conf[WRITE][off]);
+		seq_printf(sf, "%u", tg->iops_conf[WRITE][off]);
+	else
+		seq_puts(sf, "max");
+
 	if (off == LIMIT_LOW) {
-		if (tg->idletime_threshold_conf == ULONG_MAX)
-			strcpy(idle_time, " idle=max");
+		seq_puts(sf, " idle=");
+		if (tg->idletime_threshold_conf != ULONG_MAX)
+			seq_printf(sf, "%lu", tg->idletime_threshold_conf);
 		else
-			snprintf(idle_time, sizeof(idle_time), " idle=%lu",
-				tg->idletime_threshold_conf);
+			seq_puts(sf, "max");
 
-		if (tg->latency_target_conf == ULONG_MAX)
-			strcpy(latency_time, " latency=max");
+		seq_puts(sf, " latency=");
+		if (tg->latency_target_conf != ULONG_MAX)
+			seq_printf(sf, "%lu", tg->latency_target_conf);
 		else
-			snprintf(latency_time, sizeof(latency_time),
-				" latency=%lu", tg->latency_target_conf);
+			seq_puts(sf, "max");
 	}
 
-	seq_printf(sf, "%s rbps=%s wbps=%s riops=%s wiops=%s%s%s\n",
-		   dname, bufs[0], bufs[1], bufs[2], bufs[3], idle_time,
-		   latency_time);
 	return 0;
 }
 
-- 
2.34.1