Skip to contentSkip to conversation

mtd: parsers: qcom: Fix kernel panic on skipped partition

PATCH Applied Ansuel Smith 2 participants 5 messages Jan 16, 2022 – Jan 23, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 mtd: parsers: qcom: Fix kernel panic on skipped partition

In the event of a skipped partition (case when the entry name is empty) the kernel panics in the cleanup function as the name entry is NULL. Rework the parser logic by first checking the real partition number and then allocate the space and set the data for the valid partitions.

The logic was also fundamentally wrong as with a skipped partition, the parts number returned was incorrect by not decreasing it for the skipped partitions.

Fixes 803eb12 ("mtd: parsers: Add Qcom SMEM parser")
---
 drivers/mtd/parsers/qcomsmempart.c | 31 ++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)
drivers/mtd/parsers/qcomsmempart.c +19 -12
index 06a818cd2433..f4fc7635c1f3 100644
@@ -58,11 +58,11 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
 			       const struct mtd_partition **pparts,
 			       struct mtd_part_parser_data *data)
 {
+	size_t len = SMEM_FLASH_PTABLE_HDR_LEN;
+	int ret, i, j, tmpparts, numparts = 0;
 	struct smem_flash_pentry *pentry;
 	struct smem_flash_ptable *ptable;
-	size_t len = SMEM_FLASH_PTABLE_HDR_LEN;
 	struct mtd_partition *parts;
-	int ret, i, numparts;
 	char *name, *c;
 
 	if (IS_ENABLED(CONFIG_MTD_SPI_NOR_USE_4K_SECTORS)
@@ -87,8 +87,8 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
 	}
 
 	/* Ensure that # of partitions is less than the max we have allocated */
-	numparts = le32_to_cpu(ptable->numparts);
-	if (numparts > SMEM_FLASH_PTABLE_MAX_PARTS_V4) {
+	tmpparts = le32_to_cpu(ptable->numparts);
+	if (tmpparts > SMEM_FLASH_PTABLE_MAX_PARTS_V4) {
 		pr_err("Partition numbers exceed the max limit\n");
 		return -EINVAL;
 	}
@@ -116,11 +116,17 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
 		return PTR_ERR(ptable);
 	}
 
+	for (i = 0; i < tmpparts; i++) {
+		pentry = &ptable->pentry[i];
+		if (pentry->name[0] != '\0')
+			numparts++;
+	}
+
 	parts = kcalloc(numparts, sizeof(*parts), GFP_KERNEL);
 	if (!parts)
 		return -ENOMEM;
 
-	for (i = 0; i < numparts; i++) {
+	for (i = 0, j = 0; i < tmpparts; i++) {
 		pentry = &ptable->pentry[i];
 		if (pentry->name[0] == '\0')
 			continue;
@@ -135,24 +141,25 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
 		for (c = name; *c != '\0'; c++)
 			*c = tolower(*c);
 
-		parts[i].name = name;
-		parts[i].offset = le32_to_cpu(pentry->offset) * mtd->erasesize;
-		parts[i].mask_flags = pentry->attr;
-		parts[i].size = le32_to_cpu(pentry->length) * mtd->erasesize;
+		parts[j].name = name;
+		parts[j].offset = le32_to_cpu(pentry->offset) * mtd->erasesize;
+		parts[j].mask_flags = pentry->attr;
+		parts[j].size = le32_to_cpu(pentry->length) * mtd->erasesize;
 		pr_debug("%d: %s offs=0x%08x size=0x%08x attr:0x%08x\n",
 			 i, pentry->name, le32_to_cpu(pentry->offset),
 			 le32_to_cpu(pentry->length), pentry->attr);
+		j++;
 	}
 
 	pr_debug("SMEM partition table found: ver: %d len: %d\n",
-		 le32_to_cpu(ptable->version), numparts);
+		 le32_to_cpu(ptable->version), tmpparts);
 	*pparts = parts;
 
 	return numparts;
 
 out_free_parts:
-	while (--i >= 0)
-		kfree(parts[i].name);
+	while (--j >= 0)
+		kfree(parts[j].name);
 	kfree(parts);
 	*pparts = NULL;
 
Signature · 1 line
2.33.1
#1 lore raw

PATCH 2/2 mtd: parsers: qcom: Fix missing free for pparts in cleanup

Mtdpart doesn't free pparts when a cleanup function is declared. Add missing free for pparts in cleanup function for smem to fix the leak.

Fixes 10f3b4d79958 ("mtd: parsers: qcom: Fix leaking of partition name")
---
 drivers/mtd/parsers/qcomsmempart.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/mtd/parsers/qcomsmempart.c +2 -0
index f4fc7635c1f3..32ddfea70142 100644
@@ -173,6 +173,8 @@ static void parse_qcomsmem_cleanup(const struct mtd_partition *pparts,
 
 	for (i = 0; i < nr_parts; i++)
 		kfree(pparts[i].name);
+
+	kfree(pparts);
 }
 
 static const struct of_device_id qcomsmem_of_match_table[] = {
Signature · 1 line
2.33.1
maintainer Applied
#2 lore raw

Re: [PATCH 2/2] mtd: parsers: qcom: Fix missing free for pparts in cleanup

Quoted 6 lines · Ansuel Smith

Mtdpart doesn't free pparts when a cleanup function is declared. Add missing free for pparts in cleanup function for smem to fix the leak.

Fixes 10f3b4d79958 ("mtd: parsers: qcom: Fix leaking of partition name")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

maintainer Applied
#3 lore raw
In reply to Ansuel Smith
Quoted 11 lines · Ansuel Smith

In the event of a skipped partition (case when the entry name is empty) the kernel panics in the cleanup function as the name entry is NULL. Rework the parser logic by first checking the real partition number and then allocate the space and set the data for the valid partitions.

The logic was also fundamentally wrong as with a skipped partition, the parts number returned was incorrect by not decreasing it for the skipped partitions.

Fixes 803eb12 ("mtd: parsers: Add Qcom SMEM parser")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

maintainer
#4 lore raw

[email protected] wrote on Sun, 23 Jan 2022 16:23:16 +0100:

Quoted 6 lines · Ansuel Smith
Quoted 5 lines

In the event of a skipped partition (case when the entry name is empty) the kernel panics in the cleanup function as the name entry is NULL. Rework the parser logic by first checking the real partition number and then allocate the space and set the data for the valid partitions.

Quoted 8 lines
Quoted 6 lines

The logic was also fundamentally wrong as with a skipped partition, the parts number returned was incorrect by not decreasing it for the skipped partitions.

Fixes 803eb12 ("mtd: parsers: Add Qcom SMEM parser")

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Both patches applied on mtd/fixes, actually.