From: Guoxin Pu
> Sent: 01 January 2024 17:51
>
> In commit 146afeb235ccec10c17ad8ea26327c0c79dbd968 ("block: use strscpy()
> to instead of strncpy()") , the length that should now represent the length
> of the string with the terminating NULL was not updated alongside the
> change.
>
> This has caused blkdevparts= definition on kernel cmdline to be not
> correctly recognized and partitions not correctly initialized, breaking any
> device relying on such partitions to boot, on stable releases since 6.6
>
> This patch fixes the lengths to contain the terminating NULL.
>
> Cc: stable@xxxxxxxxxxxxxxx # 6.6.x
> Signed-off-by: Guoxin Pu <pugokushin@xxxxxxxxx>
> ---
> block/partitions/cmdline.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c
> index c03bc105e575..c2aac5f4ab82 100644
> --- a/block/partitions/cmdline.c
> +++ b/block/partitions/cmdline.c
> @@ -79,8 +79,8 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
> goto fail;
> }
>
> - length = min_t(int, next - partdef,
> - sizeof(new_subpart->name) - 1);
> + length = min_t(int, next - partdef + 1,
> + sizeof(new_subpart->name));
> strscpy(new_subpart->name, partdef, length);
Shouldn't that be a memcpy() with the original length?
Since it looks as though there is something equivalent to:
next = strchr(partdef, ',');
just above?
Maybe with:
new_subpart->name[length] = '\0';
if the target isn't zero filled (which the strncpy() probably
relied on.)
> @@ -138,7 +138,7 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
> goto fail;
> }
>
> - length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
> + length = min_t(int, next - bdevdef + 1, sizeof(newparts->name));
> strscpy(newparts->name, bdevdef, length);
Same.
> @@ -148,8 +148,8 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
> bdevdef = next;
> next = strchr(bdevdef, ',');
>
> - length = (!next) ? (sizeof(buf) - 1) :
> - min_t(int, next - bdevdef, sizeof(buf) - 1);
> + length = (!next) ? sizeof(buf) :
> + min_t(int, next - bdevdef + 1, sizeof(buf));
>
> strscpy(buf, bdevdef, length);
Same
> @@ -262,7 +262,7 @@ static int add_part(int slot, struct cmdline_subpart *subpart,
>
> info = &state->parts[slot].info;
>
> - label_min = min_t(int, sizeof(info->volname) - 1,
> + label_min = min_t(int, sizeof(info->volname),
> sizeof(subpart->name));
> strscpy(info->volname, subpart->name, label_min);
WTF?
That only makes any sense if subpart->name might not be '\0'
terminated - which strncpy() would have handled fine (with the -1).
Otherwise what is wrong with:
strscpy(info->volname, subpart->name, sizeof (info->volname));
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
