All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: "Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Rafał Miłecki" <rafal@milecki.pl>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] mtd: mtdpart: ignore error -ENOENT from parsers on subpartitions
Date: Wed, 12 Nov 2025 11:50:36 +0100	[thread overview]
Message-ID: <87ikffikxv.fsf@bootlin.com> (raw)
In-Reply-To: <691456b8.050a0220.3c21b3.5c4c@mx.google.com> (Christian Marangi's message of "Wed, 12 Nov 2025 10:43:17 +0100")

On 12/11/2025 at 10:43:17 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:

> On Wed, Nov 12, 2025 at 10:33:25AM +0100, Miquel Raynal wrote:
>> Hi Christian,
>> 
>> On 09/11/2025 at 12:52:44 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:
>> 
>> > Commit 5c2f7727d437 ("mtd: mtdpart: check for subpartitions parsing
>> > result") introduced some kind of regression with parser on subpartitions
>> > where if a parser emits an error then the entire parsing process from the
>> > upper parser fails and partitions are deleted.
>> >
>> > Not checking for error in subpartitions was originally intended as
>> > special parser can emit error also in the case of the partition not
>> > correctly init (for example a wiped partition) or special case where the
>> > partition should be skipped due to some ENV variables externally
>> > provided (from bootloader for example)
>> >
>> > One example case is the TRX partition where, in the context of a wiped
>> > partition, returns a -ENOENT as the trx_magic is not found in the
>> > expected TRX header (as the partition is wiped)
>> 
>> I didn't had in mind this was a valid case. I am a bit puzzled because
>> it opens the breach to other special cases, but at the same time I have
>> no strong arguments to refuse this situation so let's go for it.
>> 
>
> Thanks a lot for accepting this. I checked all the parser both upstream
> and downstream and I found this ""undocumented"" pattern of returning
> -ENOENT. [1] [2] [3]
>
> For sure it's a regression, we had various device on OpenWrt that broke
> from migrating from 6.6 to 6.12. I agree there is the risk you are
> pointing out but I feel this is a good compromise to restore original
> functionality of the upstream parsers.
>
> (the other error condition are -ENOMEM or sometimes -EINVAL for parser
> header present but very wrong)
>
> [1] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/tplink_safeloader.c#L93
> [2] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/scpart.c#L170
> [3] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/ofpart_bcm4908.c#L47

Thanks for the digging. I will apply this to -next and not -fixes. It
will be slightly longer to get it backported, but this gives a bit more
time for this patch to be thought about as I plan on sending my fixes PR
in the next days.

Miquèl

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

WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: "Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Rafał Miłecki" <rafal@milecki.pl>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] mtd: mtdpart: ignore error -ENOENT from parsers on subpartitions
Date: Wed, 12 Nov 2025 11:50:36 +0100	[thread overview]
Message-ID: <87ikffikxv.fsf@bootlin.com> (raw)
In-Reply-To: <691456b8.050a0220.3c21b3.5c4c@mx.google.com> (Christian Marangi's message of "Wed, 12 Nov 2025 10:43:17 +0100")

On 12/11/2025 at 10:43:17 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:

> On Wed, Nov 12, 2025 at 10:33:25AM +0100, Miquel Raynal wrote:
>> Hi Christian,
>> 
>> On 09/11/2025 at 12:52:44 +01, Christian Marangi <ansuelsmth@gmail.com> wrote:
>> 
>> > Commit 5c2f7727d437 ("mtd: mtdpart: check for subpartitions parsing
>> > result") introduced some kind of regression with parser on subpartitions
>> > where if a parser emits an error then the entire parsing process from the
>> > upper parser fails and partitions are deleted.
>> >
>> > Not checking for error in subpartitions was originally intended as
>> > special parser can emit error also in the case of the partition not
>> > correctly init (for example a wiped partition) or special case where the
>> > partition should be skipped due to some ENV variables externally
>> > provided (from bootloader for example)
>> >
>> > One example case is the TRX partition where, in the context of a wiped
>> > partition, returns a -ENOENT as the trx_magic is not found in the
>> > expected TRX header (as the partition is wiped)
>> 
>> I didn't had in mind this was a valid case. I am a bit puzzled because
>> it opens the breach to other special cases, but at the same time I have
>> no strong arguments to refuse this situation so let's go for it.
>> 
>
> Thanks a lot for accepting this. I checked all the parser both upstream
> and downstream and I found this ""undocumented"" pattern of returning
> -ENOENT. [1] [2] [3]
>
> For sure it's a regression, we had various device on OpenWrt that broke
> from migrating from 6.6 to 6.12. I agree there is the risk you are
> pointing out but I feel this is a good compromise to restore original
> functionality of the upstream parsers.
>
> (the other error condition are -ENOMEM or sometimes -EINVAL for parser
> header present but very wrong)
>
> [1] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/tplink_safeloader.c#L93
> [2] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/scpart.c#L170
> [3] https://elixir.bootlin.com/linux/v6.17.7/source/drivers/mtd/parsers/ofpart_bcm4908.c#L47

Thanks for the digging. I will apply this to -next and not -fixes. It
will be slightly longer to get it backported, but this gives a bit more
time for this patch to be thought about as I plan on sending my fixes PR
in the next days.

Miquèl

  reply	other threads:[~2025-11-12 10:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-09 11:52 [PATCH] mtd: mtdpart: ignore error -ENOENT from parsers on subpartitions Christian Marangi
2025-11-09 11:52 ` Christian Marangi
2025-11-12  9:33 ` Miquel Raynal
2025-11-12  9:33   ` Miquel Raynal
2025-11-12  9:43   ` Christian Marangi
2025-11-12  9:43     ` Christian Marangi
2025-11-12 10:50     ` Miquel Raynal [this message]
2025-11-12 10:50       ` Miquel Raynal
2025-11-17 10:54 ` Miquel Raynal
2025-11-17 10:54   ` Miquel Raynal

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=87ikffikxv.fsf@bootlin.com \
    --to=miquel.raynal@bootlin.com \
    --cc=ansuelsmth@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=rafal@milecki.pl \
    --cc=richard@nod.at \
    --cc=stable@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.