[PATCH v1 0/4] powerpc: assorted fixes (gpci, imc, energy, iommu)

[PATCH v1 0/4] powerpc: assorted fixes (gpci, imc, energy, iommu)

[Xixin Liu]

Hi,

This series fixes some bugs in powerpc paths found by static review
of linux-next:

  1) hv-gpci sysfs hex formatting can overflow the PAGE_SIZE show
     buffer; switch to sysfs_emit_at() and return -EFBIG
  2) opal-imc debugfs names use char[16] with sprintf for a u32 id
  3) pseries energy walks H_BEST_ENERGY cnt and sprintf()s without
     page bounds
  4) iommu debugfs uses char[10] with %08lx while it_index is
     unsigned long

Patches are independent. Please review.

Thanks,
Xixin Liu

---

Xixin Liu (4):
  powerpc/perf: hv-gpci: bound sysfs hex formatting to PAGE_SIZE
  powerpc/powernv: opal-imc: fix debugfs name buffer size
  powerpc/pseries: energy: bound H_BEST_ENERGY cnt and sysfs output
  powerpc/iommu: fix debugfs name buffer for 64-bit it_index

 arch/powerpc/kernel/iommu.c                     |  9 +++--
 arch/powerpc/perf/hv-gpci.c                     | 77 +++++++++++++++---------
 arch/powerpc/platforms/powernv/opal-imc.c       |  7 +++--
 arch/powerpc/platforms/pseries/pseries_energy.c | 16 ++++-
 4 files changed, 75 insertions(+), 34 deletions(-)

-- 
2.43.0

[PATCH v1 4/4] powerpc/iommu: fix debugfs name buffer for 64-bit it_index

[Xixin Liu]

iommu debugfs names sprintf() "%08lx" into char name[10]. The "8" in
%08lx is a minimum width, so a 64-bit it_index can need up to 16 hex
digits plus NUL.

Size the buffer for an unsigned long and use snprintf.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index ee1b5cb557c9..e8f48aef7832 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -53,10 +53,11 @@
 
 static void iommu_debugfs_add(struct iommu_table *tbl)
 {
-	char name[10];
+	/* unsigned long hex + NUL; %08lx is a minimum width only */
+	char name[2 * sizeof(unsigned long) + 1];
 	struct dentry *liobn_entry;
 
-	sprintf(name, "%08lx", tbl->it_index);
+	snprintf(name, sizeof(name), "%08lx", tbl->it_index);
 	liobn_entry = debugfs_create_dir(name, iommu_debugfs_dir);
 
 	debugfs_create_file_unsafe("weight", 0400, liobn_entry, tbl, &iommu_debugfs_fops_weight);
@@ -70,9 +71,9 @@
 
 static void iommu_debugfs_del(struct iommu_table *tbl)
 {
-	char name[10];
+	char name[2 * sizeof(unsigned long) + 1];
 
-	sprintf(name, "%08lx", tbl->it_index);
+	snprintf(name, sizeof(name), "%08lx", tbl->it_index);
 	debugfs_lookup_and_remove(name, iommu_debugfs_dir);
 }
 #else
-- 
2.43.0

[PATCH v1 1/4] powerpc/perf: hv-gpci: bound sysfs hex formatting to PAGE_SIZE

[Xixin Liu]

hv-gpci sysfs show paths format hypervisor counter bytes with sprintf()
into a PAGE_SIZE buffer, and only sometimes check the length afterwards.

Each byte becomes two hex digits plus newlines, so the output can grow
past PAGE_SIZE. Checking after sprintf() is too late: the write already
overflowed the sysfs buffer. One path had no size check at all.

Use sysfs_emit_at() so formatting stays within the sysfs buffer. If a
field cannot fit completely, return -EFBIG rather than silently
truncating.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 arch/powerpc/perf/hv-gpci.c | 77 +++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 25 deletions(-)

diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
index 76495744f..14a4f414b 100644
--- a/arch/powerpc/perf/hv-gpci.c
+++ b/arch/powerpc/perf/hv-gpci.c
@@ -136,6 +136,7 @@ static unsigned long systeminfo_gpci_request(u32 req, u32 starting_index,
 {
 	unsigned long ret;
 	size_t i, j;
+	int len;
 
 	arg->params.counter_request = cpu_to_be32(req);
 	arg->params.starting_index = cpu_to_be32(starting_index);
@@ -177,17 +178,23 @@ static unsigned long systeminfo_gpci_request(u32 req, u32 starting_index,
 	for (i = 0; i < be16_to_cpu(arg->params.returned_values); i++) {
 		j = i * be16_to_cpu(arg->params.cv_element_size);
 
-		for (; j < (i + 1) * be16_to_cpu(arg->params.cv_element_size); j++)
-			*n += sprintf(buf + *n,  "%02x", (u8)arg->bytes[j]);
-		*n += sprintf(buf + *n,  "\n");
-	}
-
-	if (*n >= PAGE_SIZE) {
-		pr_info("System information exceeds PAGE_SIZE\n");
-		return -EFBIG;
+		for (; j < (i + 1) * be16_to_cpu(arg->params.cv_element_size); j++) {
+			len = sysfs_emit_at(buf, *n, "%02x", (u8)arg->bytes[j]);
+			if (len != 2)
+				goto emit_failed;
+			*n += len;
+		}
+		len = sysfs_emit_at(buf, *n, "\n");
+		if (len != 1)
+			goto emit_failed;
+		*n += len;
 	}
 
 	return ret;
+
+emit_failed:
+	pr_info("System information does not fit in sysfs buffer\n");
+	return -EFBIG;
 }
 
 static ssize_t processor_bus_topology_show(struct device *dev, struct device_attribute *attr,
@@ -470,12 +477,13 @@ static ssize_t affinity_domain_via_domain_show(struct device *dev, struct device
 	return ret;
 }
 
-static void affinity_domain_via_partition_result_parse(int returned_values,
+static int affinity_domain_via_partition_result_parse(int returned_values,
 			int element_size, char *buf, size_t *last_element,
 			size_t *n, struct hv_gpci_request_buffer *arg)
 {
 	size_t i = 0, j = 0;
 	size_t k, l, m;
+	int len;
 	uint16_t total_affinity_domain_ele, size_of_each_affinity_domain_ele;
 
 	/*
@@ -492,27 +500,44 @@ static void affinity_domain_via_partition_result_parse(int returned_values,
 	 */
 	while (i < returned_values) {
 		k = j;
-		for (; k < j + element_size; k++)
-			*n += sprintf(buf + *n,  "%02x", (u8)arg->bytes[k]);
-		*n += sprintf(buf + *n,  "\n");
+		for (; k < j + element_size; k++) {
+			len = sysfs_emit_at(buf, *n, "%02x", (u8)arg->bytes[k]);
+			if (len != 2)
+				return -EFBIG;
+			*n += len;
+		}
+		len = sysfs_emit_at(buf, *n, "\n");
+		if (len != 1)
+			return -EFBIG;
+		*n += len;
 
 		total_affinity_domain_ele = (u8)arg->bytes[k - 2] << 8 | (u8)arg->bytes[k - 3];
 		size_of_each_affinity_domain_ele = (u8)arg->bytes[k] << 8 | (u8)arg->bytes[k - 1];
 
 		for (l = 0; l < total_affinity_domain_ele; l++) {
 			for (m = 0; m < size_of_each_affinity_domain_ele; m++) {
-				*n += sprintf(buf + *n,  "%02x", (u8)arg->bytes[k]);
+				len = sysfs_emit_at(buf, *n, "%02x", (u8)arg->bytes[k]);
+				if (len != 2)
+					return -EFBIG;
+				*n += len;
 				k++;
 			}
-			*n += sprintf(buf + *n,  "\n");
+			len = sysfs_emit_at(buf, *n, "\n");
+			if (len != 1)
+				return -EFBIG;
+			*n += len;
 		}
 
-		*n += sprintf(buf + *n,  "\n");
+		len = sysfs_emit_at(buf, *n, "\n");
+		if (len != 1)
+			return -EFBIG;
+		*n += len;
 		i++;
 		j = k;
 	}
 
 	*last_element = k;
+	return 0;
 }
 
 static ssize_t affinity_domain_via_partition_show(struct device *dev, struct device_attribute *attr,
@@ -555,12 +580,10 @@ static ssize_t affinity_domain_via_partition_show(struct device *dev, struct dev
 	 * to buffer util we get all the information.
 	 */
 	while (ret == H_PARAMETER) {
-		affinity_domain_via_partition_result_parse(
-			be16_to_cpu(arg->params.returned_values) - 1,
-			be16_to_cpu(arg->params.cv_element_size), buf,
-			&last_element, &n, arg);
-
-		if (n >= PAGE_SIZE) {
+		if (affinity_domain_via_partition_result_parse(
+				be16_to_cpu(arg->params.returned_values) - 1,
+				be16_to_cpu(arg->params.cv_element_size), buf,
+				&last_element, &n, arg)) {
 			put_cpu_var(hv_gpci_reqb);
 			pr_debug("System information does not fit in sysfs buffer\n");
 			return -EFBIG;
@@ -587,10 +610,14 @@ static ssize_t affinity_domain_via_partition_show(struct device *dev, struct dev
 	}
 
 parse_result:
-	affinity_domain_via_partition_result_parse(
-		be16_to_cpu(arg->params.returned_values),
-		be16_to_cpu(arg->params.cv_element_size),
-		buf, &last_element, &n, arg);
+	if (affinity_domain_via_partition_result_parse(
+			be16_to_cpu(arg->params.returned_values),
+			be16_to_cpu(arg->params.cv_element_size),
+			buf, &last_element, &n, arg)) {
+		put_cpu_var(hv_gpci_reqb);
+		pr_debug("System information does not fit in sysfs buffer\n");
+		return -EFBIG;
+	}
 
 	put_cpu_var(hv_gpci_reqb);
 	return n;
-- 
2.43.0

[PATCH v1 2/4] powerpc/powernv: opal-imc: fix debugfs name buffer size

[Xixin Liu]

char mode[16]/cmd[16] are too small for sprintf("imc_mode_%d") /
sprintf("imc_cmd_%d") when id is a full u32 (up to 20 bytes including NUL).

id comes from DT "chip-id" as u32. Enlarge the buffers and use snprintf.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index b3fd5c648dea..77de53d47cb6 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -51,7 +51,8 @@
 				    struct imc_pmu *pmu_ptr)
 {
 	static u64 loc, *imc_mode_addr, *imc_cmd_addr;
-	char mode[16], cmd[16];
+	/* "imc_mode_" / "imc_cmd_" + max u32 decimal + NUL */
+	char mode[20], cmd[20];
 	u32 cb_offset;
 	struct imc_mem_info *ptr = pmu_ptr->mem_info;
 
@@ -63,12 +64,12 @@
 	while (ptr->vbase != NULL) {
 		loc = (u64)(ptr->vbase) + cb_offset;
 		imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
-		sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
+		snprintf(mode, sizeof(mode), "imc_mode_%u", ptr->id);
 		imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
 				       imc_mode_addr);
 
 		imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
-		sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
+		snprintf(cmd, sizeof(cmd), "imc_cmd_%u", ptr->id);
 		imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
 				       imc_cmd_addr);
 		ptr++;
-- 
2.43.0

[PATCH v1 3/4] powerpc/pseries: energy: bound H_BEST_ENERGY cnt and sysfs output

[Xixin Liu]

The H_BEST_ENERGY sysfs path takes cnt from the hypercall return buffer
and walks buf_page[2*i+1] without checking that cnt fits in the single
page allocated for the hcall, and sprintf()s into the PAGE_SIZE sysfs
buffer without remaining-space checks.

Clamp cnt to the number of u32 pairs that fit in the page, and stop
formatting before overflowing the sysfs page.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pseries_energy.c b/arch/powerpc/platforms/pseries/pseries_energy.c
index fdaf85ecd39b..8344f00656e5 100644
--- a/arch/powerpc/platforms/pseries/pseries_energy.c
+++ b/arch/powerpc/platforms/pseries/pseries_energy.c
@@ -209,16 +209,28 @@
 		return -EINVAL;
 	}
 
+	/*
+	 * Each entry occupies two u32s in buf_page. Never walk past the
+	 * page, and never sprintf past the PAGE_SIZE sysfs buffer.
+	 */
 	cnt = retbuf[0];
+	if (cnt > (PAGE_SIZE / sizeof(u32)) / 2)
+		cnt = (PAGE_SIZE / sizeof(u32)) / 2;
+
 	for (i = 0; i < cnt; i++) {
 		cpu = drc_index_to_cpu(buf_page[2*i+1]);
 		if ((cpu_online(cpu) && !activate) ||
-		    (!cpu_online(cpu) && activate))
+		    (!cpu_online(cpu) && activate)) {
+			if (s - page >= PAGE_SIZE - 16)
+				break;
 			s += sprintf(s, "%d,", cpu);
+		}
 	}
 	if (s > page) { /* Something to show */
 		s--; /* Suppress last comma */
-		s += sprintf(s, "\n");
+		/* sprintf needs room for '\n' and trailing NUL. */
+		if (s - page < PAGE_SIZE - 1)
+			s += sprintf(s, "\n");
 	}
 
 	free_page((unsigned long) buf_page);
-- 
2.43.0