Linux-mtd Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Gabor Juhos" <j4g8y7@gmail.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org
Subject: Re: [PATCH v2] mtd: parsers: qcom: drop setting of partition's mask_flags
Date: Tue, 13 Oct 2026 10:41:14 +0000	[thread overview]
Message-ID: <sashiko-outbox-169529@kernel.org> (raw)
In-Reply-To: <20261013-qcom-smempart-ignore-mask_flags-v2-1-01b7e133f273@gmail.com>

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/

      reply	other threads:[~2026-10-13 10:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-169529@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=j4g8y7@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox