RE: [PATCH] block: fix length of strscpy()

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@vger.kernel.org # 6.6.x Signed-off-by: Guoxin Pu pugokushin@gmail.com

---

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)