Received-SPF: pass (google.com: domain of linux-kernel+bounces-167334-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: David Yang <mmyangfl@gmail.com>
To: linux-block@vger.kernel.org
Cc: David Yang <mmyangfl@gmail.com>,
	Jens Axboe <axboe@kernel.dk>,
	Xu Panda <xu.panda@zte.com.cn>,
	Justin Stitt <justinstitt@google.com>,
	Yang Yang <yang.yang29@zte.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] block: fix buf size for strscpy()
Date: Fri,  3 May 2024 15:48:37 +0800
Message-ID: <20240503074845.12181-1-mmyangfl@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

strscpy() takes the total size of destination buffer as the argument,
including the space for the terminating null character.

The actual length of the buffer should be len(str) + 1, which can be
seen from the indexes where null characters are written in the code
before the commit in question, and 'sizeof(buf) - 1' right above
the problematic codes.

Without the additional 1 size and the absence of checkes against -E2BIG,
strscpy() will angrily eat the last character of the source string. In
my situation, strscpy() will take away one character before the comma
"," (which is presumably the right bracket ")") in parse_parts(), making
parse_subpart() unable to 'strchr(++partdef, ')')' and producing the
following error message:

  cmdline partition format is invalid.

Fixes: 146afeb235cc ("block: use strscpy() to instead of strncpy()")
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 block/partitions/cmdline.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/partitions/cmdline.c b/block/partitions/cmdline.c
index c03bc105e575..6d8401ce2943 100644
--- a/block/partitions/cmdline.c
+++ b/block/partitions/cmdline.c
@@ -81,7 +81,7 @@ static int parse_subpart(struct cmdline_subpart **subpart, char *partdef)
 
 		length = min_t(int, next - partdef,
 			       sizeof(new_subpart->name) - 1);
-		strscpy(new_subpart->name, partdef, length);
+		strscpy(new_subpart->name, partdef, length + 1);
 
 		partdef = ++next;
 	} else
@@ -139,7 +139,7 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
 	}
 
 	length = min_t(int, next - bdevdef, sizeof(newparts->name) - 1);
-	strscpy(newparts->name, bdevdef, length);
+	strscpy(newparts->name, bdevdef, length + 1);
 	newparts->nr_subparts = 0;
 
 	next_subpart = &newparts->subpart;
@@ -151,7 +151,7 @@ static int parse_parts(struct cmdline_parts **parts, const char *bdevdef)
 		length = (!next) ? (sizeof(buf) - 1) :
 			min_t(int, next - bdevdef, sizeof(buf) - 1);
 
-		strscpy(buf, bdevdef, length);
+		strscpy(buf, bdevdef, length + 1);
 
 		ret = parse_subpart(next_subpart, buf);
 		if (ret)
@@ -264,7 +264,7 @@ static int add_part(int slot, struct cmdline_subpart *subpart,
 
 	label_min = min_t(int, sizeof(info->volname) - 1,
 			  sizeof(subpart->name));
-	strscpy(info->volname, subpart->name, label_min);
+	strscpy(info->volname, subpart->name, label_min + 1);
 
 	snprintf(tmp, sizeof(tmp), "(%s)", info->volname);
 	strlcat(state->pp_buf, tmp, PAGE_SIZE);
-- 
2.43.0