forked from openwrt/openwrt
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <[email protected]>
- Loading branch information
Showing
1 changed file
with
217 additions
and
0 deletions.
There are no files selected for viewing
217 changes: 217 additions & 0 deletions
217
...et/linux/generic/pending-6.6/195-block-fix-and-simplify-blkdevparts-cmdline-parsing.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <[email protected]> | ||
X-Patchwork-Id: 13637306 | ||
From: INAGAKI Hiroshi <[email protected]> | ||
To: [email protected] | ||
Cc: [email protected], | ||
[email protected], | ||
[email protected], | ||
[email protected], | ||
[email protected], | ||
INAGAKI Hiroshi <[email protected]>, | ||
Naohiro Aota <[email protected]> | ||
Subject: [PATCH] block: fix and simplify blkdevparts= cmdline parsing | ||
Date: Sun, 21 Apr 2024 16:39:52 +0900 | ||
Message-ID: <[email protected]> | ||
X-Mailer: git-send-email 2.42.0.windows.2 | ||
Precedence: bulk | ||
X-Mailing-List: [email protected] | ||
List-Id: <linux-block.vger.kernel.org> | ||
List-Subscribe: <mailto:[email protected]> | ||
List-Unsubscribe: <mailto:[email protected]> | ||
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@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@10M(kernel-1),..." is passed to strscpy() with length=17 | ||
from parse_parts() | ||
2. strscpy() returns -E2BIG and the destination buffer has | ||
"48M@10M(kernel-1\0" | ||
3. "48M@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@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 <[email protected]> | ||
Signed-off-by: INAGAKI Hiroshi <[email protected]> | ||
Reviewed-by: Daniel Golle <[email protected]> | ||
--- | ||
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); |