* [PATCH] mtd: parser: cmdline: Fix parsing of part-names with colons
@ 2020-11-22 0:15 Sven Eckelmann
2020-11-23 8:38 ` Miquel Raynal
0 siblings, 1 reply; 3+ messages in thread
From: Sven Eckelmann @ 2020-11-22 0:15 UTC (permalink / raw)
To: linux-mtd
Cc: vigneshr, richard, rminnich, boris.brezillon, miquel.raynal,
Sven Eckelmann
Some devices (especially QCA ones) are already using hardcoded partition
names with colons in it. The OpenMesh A62 for example provides following
mtd relevant information via cmdline:
root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait
The change to split only on the last colon between mtd-id and partitions
will cause newpart to see following string for the first partition:
KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive)
Such a partition list cannot be parsed and thus the device fails to boot.
Avoid this behavior by making sure that the start of the first part-name
("(") will also be the last byte the mtd-id split algorithm is using for
its colon search.
Fixes: eb13fa022741 ("mtd: parser: cmdline: Support MTD names containing one or more colons")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
drivers/mtd/parsers/cmdlinepart.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/parsers/cmdlinepart.c b/drivers/mtd/parsers/cmdlinepart.c
index a79e4d866b08..655c6d59bd05 100644
--- a/drivers/mtd/parsers/cmdlinepart.c
+++ b/drivers/mtd/parsers/cmdlinepart.c
@@ -226,7 +226,7 @@ static int mtdpart_setup_real(char *s)
struct cmdline_mtd_partition *this_mtd;
struct mtd_partition *parts;
int mtd_id_len, num_parts;
- char *p, *mtd_id, *semicol;
+ char *p, *mtd_id, *semicol, *open_parenth;
/*
* Replace the first ';' by a NULL char so strrchr can work
@@ -236,6 +236,13 @@ static int mtdpart_setup_real(char *s)
if (semicol)
*semicol = '\0';
+ /* make sure that part-names with ":" will not be handled as
+ * part of the mtd-id with an ":"
+ */
+ open_parenth = strchr(s, '(');
+ if (open_parenth)
+ *open_parenth = '\0';
+
mtd_id = s;
/*
@@ -245,6 +252,10 @@ static int mtdpart_setup_real(char *s)
*/
p = strrchr(s, ':');
+ /* Restore the '(' now. */
+ if (open_parenth)
+ *open_parenth = '(';
+
/* Restore the ';' now. */
if (semicol)
*semicol = ';';
--
2.29.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Fix parsing of part-names with colons
2020-11-22 0:15 [PATCH] mtd: parser: cmdline: Fix parsing of part-names with colons Sven Eckelmann
@ 2020-11-23 8:38 ` Miquel Raynal
2020-11-23 8:57 ` Boris Brezillon
0 siblings, 1 reply; 3+ messages in thread
From: Miquel Raynal @ 2020-11-23 8:38 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: richard, boris.brezillon, linux-mtd, vigneshr, rminnich
Hi Sven,
Sven Eckelmann <sven@narfation.org> wrote on Sun, 22 Nov 2020 01:15:33
+0100:
> Some devices (especially QCA ones) are already using hardcoded partition
> names with colons in it. The OpenMesh A62 for example provides following
> mtd relevant information via cmdline:
>
> root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait
>
> The change to split only on the last colon between mtd-id and partitions
> will cause newpart to see following string for the first partition:
>
> KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive)
>
> Such a partition list cannot be parsed and thus the device fails to boot.
>
> Avoid this behavior by making sure that the start of the first part-name
> ("(") will also be the last byte the mtd-id split algorithm is using for
> its colon search.
Nice.
Ron, can you give this patch a shot and answer with your Tested-by: to
this thread if it still works for you?
> Fixes: eb13fa022741 ("mtd: parser: cmdline: Support MTD names containing one or more colons")
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> drivers/mtd/parsers/cmdlinepart.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/parsers/cmdlinepart.c b/drivers/mtd/parsers/cmdlinepart.c
> index a79e4d866b08..655c6d59bd05 100644
> --- a/drivers/mtd/parsers/cmdlinepart.c
> +++ b/drivers/mtd/parsers/cmdlinepart.c
> @@ -226,7 +226,7 @@ static int mtdpart_setup_real(char *s)
> struct cmdline_mtd_partition *this_mtd;
> struct mtd_partition *parts;
> int mtd_id_len, num_parts;
> - char *p, *mtd_id, *semicol;
> + char *p, *mtd_id, *semicol, *open_parenth;
>
> /*
> * Replace the first ';' by a NULL char so strrchr can work
> @@ -236,6 +236,13 @@ static int mtdpart_setup_real(char *s)
> if (semicol)
> *semicol = '\0';
>
> + /* make sure that part-names with ":" will not be handled as
> + * part of the mtd-id with an ":"
We prefer this style of multiline comment in MTD:
/*
* Something.
* Something else.
*/
> + */
> + open_parenth = strchr(s, '(');
> + if (open_parenth)
> + *open_parenth = '\0';
> +
> mtd_id = s;
>
> /*
> @@ -245,6 +252,10 @@ static int mtdpart_setup_real(char *s)
> */
> p = strrchr(s, ':');
>
> + /* Restore the '(' now. */
> + if (open_parenth)
> + *open_parenth = '(';
> +
> /* Restore the ';' now. */
> if (semicol)
> *semicol = ';';
Otherwise LGTM.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mtd: parser: cmdline: Fix parsing of part-names with colons
2020-11-23 8:38 ` Miquel Raynal
@ 2020-11-23 8:57 ` Boris Brezillon
0 siblings, 0 replies; 3+ messages in thread
From: Boris Brezillon @ 2020-11-23 8:57 UTC (permalink / raw)
To: Miquel Raynal; +Cc: richard, linux-mtd, vigneshr, Sven Eckelmann, rminnich
On Mon, 23 Nov 2020 09:38:14 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Sven,
>
> Sven Eckelmann <sven@narfation.org> wrote on Sun, 22 Nov 2020 01:15:33
> +0100:
>
> > Some devices (especially QCA ones) are already using hardcoded partition
> > names with colons in it. The OpenMesh A62 for example provides following
> > mtd relevant information via cmdline:
> >
> > root=31:11 mtdparts=spi0.0:256k(0:SBL1),128k(0:MIBIB),384k(0:QSEE),64k(0:CDT),64k(0:DDRPARAMS),64k(0:APPSBLENV),512k(0:APPSBL),64k(0:ART),64k(custom),64k(0:KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive) rootfsname=rootfs rootwait
> >
> > The change to split only on the last colon between mtd-id and partitions
> > will cause newpart to see following string for the first partition:
> >
> > KEYS),0x002b0000(kernel),0x00c80000(rootfs),15552k(inactive)
> >
> > Such a partition list cannot be parsed and thus the device fails to boot.
> >
> > Avoid this behavior by making sure that the start of the first part-name
> > ("(") will also be the last byte the mtd-id split algorithm is using for
> > its colon search.
Sounds reasonable to me. Note that it works until people start using
parens in their MTD name, so let's hope nobody does that :D. We should
really rework the mtdpart parsing code to make it more robust (do
context based parsing instead of this simple 'strchr()' solution).
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-23 8:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-22 0:15 [PATCH] mtd: parser: cmdline: Fix parsing of part-names with colons Sven Eckelmann
2020-11-23 8:38 ` Miquel Raynal
2020-11-23 8:57 ` Boris Brezillon
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.