[openwrt/openwrt] generic: 6.6: fix and simplify blkdevparts= cmdline parsing
LEDE Commits
lede-commits at lists.infradead.org
Fri May 3 15:02:51 PDT 2024
dangole pushed a commit to openwrt/openwrt.git, branch main:
https://git.openwrt.org/fb2475e6bd5dc8cc5cd50c8941d75d9932a15579
commit fb2475e6bd5dc8cc5cd50c8941d75d9932a15579
Author: Daniel Golle <daniel at makrotopia.org>
AuthorDate: Fri May 3 21:27:37 2024 +0100
generic: 6.6: fix and simplify blkdevparts= cmdline parsing
Import pending patch to fix the cmdline parsing of the "blkdevparts="
parameter which has been broken somewhen between Linux 6.1 and Linux 6.6.
Signed-off-by: Daniel Golle <daniel at makrotopia.org>
---
...-and-simplify-blkdevparts-cmdline-parsing.patch | 217 +++++++++++++++++++++
1 file changed, 217 insertions(+)
diff --git a/target/linux/generic/pending-6.6/195-block-fix-and-simplify-blkdevparts-cmdline-parsing.patch b/target/linux/generic/pending-6.6/195-block-fix-and-simplify-blkdevparts-cmdline-parsing.patch
new file mode 100644
index 0000000000..d504a74fde
--- /dev/null
+++ b/target/linux/generic/pending-6.6/195-block-fix-and-simplify-blkdevparts-cmdline-parsing.patch
@@ -0,0 +1,217 @@
+From patchwork Sun Apr 21 07:39:52 2024
+Content-Type: text/plain; charset="utf-8"
+MIME-Version: 1.0
+Content-Transfer-Encoding: 7bit
+X-Patchwork-Submitter: INAGAKI Hiroshi <musashino.open at gmail.com>
+X-Patchwork-Id: 13637306
+From: INAGAKI Hiroshi <musashino.open at gmail.com>
+To: axboe at kernel.dk
+Cc: yang.yang29 at zte.com,
+ justinstitt at google.com,
+ xu.panda at zte.com.cn,
+ linux-block at vger.kernel.org,
+ linux-kernel at vger.kernel.org,
+ INAGAKI Hiroshi <musashino.open at gmail.com>,
+ Naohiro Aota <naota at elisp.net>
+Subject: [PATCH] block: fix and simplify blkdevparts= cmdline parsing
+Date: Sun, 21 Apr 2024 16:39:52 +0900
+Message-ID: <20240421074005.565-1-musashino.open at gmail.com>
+X-Mailer: git-send-email 2.42.0.windows.2
+Precedence: bulk
+X-Mailing-List: linux-block at vger.kernel.org
+List-Id: <linux-block.vger.kernel.org>
+List-Subscribe: <mailto:linux-block+subscribe at vger.kernel.org>
+List-Unsubscribe: <mailto:linux-block+unsubscribe at vger.kernel.org>
+MIME-Version: 1.0
+
+Fix the cmdline parsing of the "blkdevparts=" parameter using strsep(),
+which makes the code simpler.
+
+Before commit 146afeb235cc ("block: use strscpy() to instead of
+strncpy()"), we used a strncpy() to copy a block device name and partition
+names. The commit simply replaced a strncpy() and NULL termination with
+a strscpy(). It did not update calculations of length passed to strscpy().
+While the length passed to strncpy() is just a length of valid characters
+without NULL termination ('\0'), strscpy() takes it as a length of the
+destination buffer, including a NULL termination.
+
+Since the source buffer is not necessarily NULL terminated, the current
+code copies "length - 1" characters and puts a NULL character in the
+destination buffer. It replaces the last character with NULL and breaks
+the parsing.
+
+As an example, that buffer will be passed to parse_parts() and breaks
+parsing sub-partitions due to the missing ')' at the end, like the
+following.
+
+example (Check Point V-80 & OpenWrt):
+
+- Linux Kernel 6.6
+
+ [ 0.000000] Kernel command line: console=ttyS0,115200 earlycon=uart8250,mmio32,0xf0512000 crashkernel=30M mvpp2x.queue_mode=1 blkdevparts=mmcblk1:48M at 10M(kernel-1),1M(dtb-1),720M(rootfs-1),48M(kernel-2),1M(dtb-2),720M(rootfs-2),300M(default_sw),650M(logs),1M(preset_cfg),1M(adsl),-(storage) maxcpus=4
+ ...
+ [ 0.884016] mmc1: new HS200 MMC card at address 0001
+ [ 0.889951] mmcblk1: mmc1:0001 004GA0 3.69 GiB
+ [ 0.895043] cmdline partition format is invalid.
+ [ 0.895704] mmcblk1: p1
+ [ 0.903447] mmcblk1boot0: mmc1:0001 004GA0 2.00 MiB
+ [ 0.908667] mmcblk1boot1: mmc1:0001 004GA0 2.00 MiB
+ [ 0.913765] mmcblk1rpmb: mmc1:0001 004GA0 512 KiB, chardev (248:0)
+
+ 1. "48M at 10M(kernel-1),..." is passed to strscpy() with length=17
+ from parse_parts()
+ 2. strscpy() returns -E2BIG and the destination buffer has
+ "48M at 10M(kernel-1\0"
+ 3. "48M at 10M(kernel-1\0" is passed to parse_subpart()
+ 4. parse_subpart() fails to find ')' when parsing a partition name,
+ and returns error
+
+- Linux Kernel 6.1
+
+ [ 0.000000] Kernel command line: console=ttyS0,115200 earlycon=uart8250,mmio32,0xf0512000 crashkernel=30M mvpp2x.queue_mode=1 blkdevparts=mmcblk1:48M at 10M(kernel-1),1M(dtb-1),720M(rootfs-1),48M(kernel-2),1M(dtb-2),720M(rootfs-2),300M(default_sw),650M(logs),1M(preset_cfg),1M(adsl),-(storage) maxcpus=4
+ ...
+ [ 0.953142] mmc1: new HS200 MMC card at address 0001
+ [ 0.959114] mmcblk1: mmc1:0001 004GA0 3.69 GiB
+ [ 0.964259] mmcblk1: p1(kernel-1) p2(dtb-1) p3(rootfs-1) p4(kernel-2) p5(dtb-2) 6(rootfs-2) p7(default_sw) p8(logs) p9(preset_cfg) p10(adsl) p11(storage)
+ [ 0.979174] mmcblk1boot0: mmc1:0001 004GA0 2.00 MiB
+ [ 0.984674] mmcblk1boot1: mmc1:0001 004GA0 2.00 MiB
+ [ 0.989926] mmcblk1rpmb: mmc1:0001 004GA0 512 KiB, chardev (248:0
+
+By the way, strscpy() takes a length of destination buffer and it is
+often confusing when copying characters with a specified length. Using
+strsep() helps to separate the string by the specified character. Then,
+we can use strscpy() naturally with the size of the destination buffer.
+
+Separating the string on the fly is also useful to omit the redundant
+string copy, reducing memory usage and improve the code readability.
+
+Fixes: 146afeb235cc ("block: use strscpy() to instead of strncpy()")
+Suggested-by: Naohiro Aota <naota at elisp.net>
+Signed-off-by: INAGAKI Hiroshi <musashino.open at gmail.com>
+Reviewed-by: Daniel Golle <daniel at makrotopia.org>
+---
+ block/partitions/cmdline.c | 49 ++++++++++----------------------------
+ 1 file changed, 12 insertions(+), 37 deletions(-)
+
+--- a/block/partitions/cmdline.c
++++ b/block/partitions/cmdline.c
+@@ -70,8 +70,8 @@ static int parse_subpart(struct cmdline_
+ }
+
+ if (*partdef == '(') {
+- int length;
+- char *next = strchr(++partdef, ')');
++ partdef++;
++ char *next = strsep(&partdef, ")");
+
+ if (!next) {
+ pr_warn("cmdline partition format is invalid.");
+@@ -79,11 +79,7 @@ static int parse_subpart(struct cmdline_
+ goto fail;
+ }
+
+- length = min_t(int, next - partdef,
+- sizeof(new_subpart->name) - 1);
+- strscpy(new_subpart->name, partdef, length);
+-
+- partdef = ++next;
++ strscpy(new_subpart->name, next, sizeof(new_subpart->name));
+ } else
+ new_subpart->name[0] = '\0';
+
+@@ -117,14 +113,12 @@ static void free_subpart(struct cmdline_
+ }
+ }
+
+-static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
++static int parse_parts(struct cmdline_parts **parts, char *bdevdef)
+ {
+ int ret = -EINVAL;
+ char *next;
+- int length;
+ struct cmdline_subpart **next_subpart;
+ struct cmdline_parts *newparts;
+- char buf[BDEVNAME_SIZE + 32 + 4];
+
+ *parts = NULL;
+
+@@ -132,28 +126,19 @@ static int parse_parts(struct cmdline_pa
+ if (!newparts)
+ return -ENOMEM;
+
+- next = strchr(bdevdef, ':');
++ next = strsep(&bdevdef, ":");
+ if (!next) {
+ pr_warn("cmdline partition has no block device.");
+ goto fail;
+ }
+
+- length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
+- strscpy(newparts->name, bdevdef, length);
++ strscpy(newparts->name, next, sizeof(newparts->name));
+ newparts->nr_subparts = 0;
+
+ next_subpart = &newparts->subpart;
+
+- while (next && *(++next)) {
+- bdevdef = next;
+- next = strchr(bdevdef, ',');
+-
+- length = (!next) ? (sizeof(buf) - 1) :
+- min_t(int, next - bdevdef, sizeof(buf) - 1);
+-
+- strscpy(buf, bdevdef, length);
+-
+- ret = parse_subpart(next_subpart, buf);
++ while ((next = strsep(&bdevdef, ","))) {
++ ret = parse_subpart(next_subpart, next);
+ if (ret)
+ goto fail;
+
+@@ -199,24 +184,17 @@ static int cmdline_parts_parse(struct cm
+
+ *parts = NULL;
+
+- next = pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
++ pbuf = buf = kstrdup(cmdline, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ next_parts = parts;
+
+- while (next && *pbuf) {
+- next = strchr(pbuf, ';');
+- if (next)
+- *next = '\0';
+-
+- ret = parse_parts(next_parts, pbuf);
++ while ((next = strsep(&pbuf, ";"))) {
++ ret = parse_parts(next_parts, next);
+ if (ret)
+ goto fail;
+
+- if (next)
+- pbuf = ++next;
+-
+ next_parts = &(*next_parts)->next_parts;
+ }
+
+@@ -250,7 +228,6 @@ static struct cmdline_parts *bdev_parts;
+ static int add_part(int slot, struct cmdline_subpart *subpart,
+ struct parsed_partitions *state)
+ {
+- int label_min;
+ struct partition_meta_info *info;
+ char tmp[sizeof(info->volname) + 4];
+
+@@ -262,9 +239,7 @@ static int add_part(int slot, struct cmd
+
+ info = &state->parts[slot].info;
+
+- label_min = min_t(int, sizeof(info->volname) - 1,
+- sizeof(subpart->name));
+- strscpy(info->volname, subpart->name, label_min);
++ strscpy(info->volname, subpart->name, sizeof(info->volname));
+
+ snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
+ strlcat(state->pp_buf, tmp, PAGE_SIZE);
More information about the lede-commits
mailing list