Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mtd: parsers: qcom: drop setting of partition's mask_flags
@ 2026-10-13 10:32 Gabor Juhos
  2026-10-13 10:41 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Gabor Juhos @ 2026-10-13 10:32 UTC (permalink / raw)
  To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra
  Cc: linux-arm-msm, linux-mtd, linux-kernel, Gabor Juhos

The 'attr' member of struct 'smem_flash_pentry' is 8 bits wide,
so it is only usable to mask the lower 8 bits in the MTD flags.
However the MTD layer does not define any maskable flags in
these bits at all. Additionally, the meaning of the bits in the
'attr' field is not documented, so it is not even possible to map
those to the defined MTD flags.

Due to the reasons above, remove the 'mask_flags' assignment code
since it has no effect anyway.

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - rebase on tip of mtd/next
  - Link to v1: https://lore.kernel.org/r/20261006-qcom-smempart-ignore-mask_flags-v1-1-a48d9395b21b@gmail.com
---
 drivers/mtd/parsers/qcomsmempart.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mtd/parsers/qcomsmempart.c b/drivers/mtd/parsers/qcomsmempart.c
index 1591aea48b28..355efb2e4e40 100644
--- a/drivers/mtd/parsers/qcomsmempart.c
+++ b/drivers/mtd/parsers/qcomsmempart.c
@@ -144,7 +144,6 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
 
 		parts[j].name = name;
 		parts[j].offset = (u64)le32_to_cpu(pentry->offset) * mtd->erasesize;
-		parts[j].mask_flags = pentry->attr;
 		parts[j].size = (u64)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),

---
base-commit: 2f7fa8d90694d21dce68ceabd8383252b111cfea
change-id: 20261006-qcom-smempart-ignore-mask_flags-870cc5a294d0

Best regards,
-- 
Gabor Juhos <j4g8y7@gmail.com>


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] mtd: parsers: qcom: drop setting of partition's mask_flags
  2026-10-13 10:32 [PATCH v2] mtd: parsers: qcom: drop setting of partition's mask_flags Gabor Juhos
@ 2026-10-13 10:41 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-13 10:41 UTC (permalink / raw)
  To: Gabor Juhos, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: linux-mtd

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] qcomsmempart: heap buffer overflow via SMEM TOCTOU
--

commit 9a30e6e744ea6a93af6069ce8afe6e5e1ff75040
Author: Gabor Juhos <j4g8y7@gmail.com>

mtd: parsers: qcom: drop setting of partition's mask_flags

This commit removes the assignment of mask_flags from the smem_flash_pentry
structure's attr member in the Qualcomm SMEM partition parser. The MTD
layer does not define maskable flags in the lower 8 bits, and the field is
undocumented.

Link: https://lore.kernel.org/r/20261006-qcom-smempart-ignore-mask_flags-v1-1-a48d9395b21b@gmail.com

> diff --git a/drivers/mtd/parsers/qcomsmempart.c b/drivers/mtd/parsers/qcomsmempart.c
> index 1591aea48b280..355efb2e4e40c 100644
> --- a/drivers/mtd/parsers/qcomsmempart.c
> +++ b/drivers/mtd/parsers/qcomsmempart.c
> @@ -144,7 +144,6 @@ static int parse_qcomsmem_part(struct mtd_info *mtd,
>  
>  		parts[j].name = name;
>  		parts[j].offset = (u64)le32_to_cpu(pentry->offset) * mtd->erasesize;
> -		parts[j].mask_flags = pentry->attr;
>  		parts[j].size = (u64)le32_to_cpu(pentry->length) * mtd->erasesize;

[Severity: High]
Could this loop overwrite the parts array out of bounds?

Since ptable is mapped in shared memory via qcom_smem_get(), the contents
could be modified by a remote coprocessor concurrently.

Earlier in parse_qcomsmem_part(), the code counts valid partitions:

	for (i = 0; i < tmpparts; i++) {
		pentry = &ptable->pentry[i];
		if (pentry->name[0] != '\0')
			numparts++;
	}

	parts = kzalloc_objs(*parts, numparts);

If a remote coprocessor changes pentry->name[0] from '\0' to a non-null
value between this first counting loop and the second population loop,
would the second loop find more valid entries than numparts?

This appears like it would cause the index j to increment past the allocated
size of parts, resulting in an out-of-bounds heap write.

Would it be safer to add an explicit bounds check against numparts before
assigning values to parts[j]?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-qcom-smempart-ignore-mask_flags-v2-1-01b7e133f273@gmail.com?part=1

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-13 10:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 10:32 [PATCH v2] mtd: parsers: qcom: drop setting of partition's mask_flags Gabor Juhos
2026-10-13 10:41 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox