Skip to content

Commit 5c584fe

Browse files
azeemshaikh38martinkpetersen
authored andcommitted
scsi: target: Replace strlcpy() with strscpy()
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). Direct replacement is safe here since return value of -errno is used to check for truncation instead of sizeof(dest). [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] KSPP#89 Signed-off-by: Azeem Shaikh <[email protected]> Link: https://lore.kernel.org/r/[email protected] Reviewed-by: Kees Cook <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent d0b0822 commit 5c584fe

File tree

1 file changed

+12
-12
lines changed

1 file changed

+12
-12
lines changed

drivers/target/target_core_configfs.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1392,16 +1392,16 @@ static ssize_t target_wwn_vendor_id_store(struct config_item *item,
13921392
/* +2 to allow for a trailing (stripped) '\n' and null-terminator */
13931393
unsigned char buf[INQUIRY_VENDOR_LEN + 2];
13941394
char *stripped = NULL;
1395-
size_t len;
1395+
ssize_t len;
13961396
ssize_t ret;
13971397

1398-
len = strlcpy(buf, page, sizeof(buf));
1399-
if (len < sizeof(buf)) {
1398+
len = strscpy(buf, page, sizeof(buf));
1399+
if (len > 0) {
14001400
/* Strip any newline added from userspace. */
14011401
stripped = strstrip(buf);
14021402
len = strlen(stripped);
14031403
}
1404-
if (len > INQUIRY_VENDOR_LEN) {
1404+
if (len < 0 || len > INQUIRY_VENDOR_LEN) {
14051405
pr_err("Emulated T10 Vendor Identification exceeds"
14061406
" INQUIRY_VENDOR_LEN: " __stringify(INQUIRY_VENDOR_LEN)
14071407
"\n");
@@ -1448,16 +1448,16 @@ static ssize_t target_wwn_product_id_store(struct config_item *item,
14481448
/* +2 to allow for a trailing (stripped) '\n' and null-terminator */
14491449
unsigned char buf[INQUIRY_MODEL_LEN + 2];
14501450
char *stripped = NULL;
1451-
size_t len;
1451+
ssize_t len;
14521452
ssize_t ret;
14531453

1454-
len = strlcpy(buf, page, sizeof(buf));
1455-
if (len < sizeof(buf)) {
1454+
len = strscpy(buf, page, sizeof(buf));
1455+
if (len > 0) {
14561456
/* Strip any newline added from userspace. */
14571457
stripped = strstrip(buf);
14581458
len = strlen(stripped);
14591459
}
1460-
if (len > INQUIRY_MODEL_LEN) {
1460+
if (len < 0 || len > INQUIRY_MODEL_LEN) {
14611461
pr_err("Emulated T10 Vendor exceeds INQUIRY_MODEL_LEN: "
14621462
__stringify(INQUIRY_MODEL_LEN)
14631463
"\n");
@@ -1504,16 +1504,16 @@ static ssize_t target_wwn_revision_store(struct config_item *item,
15041504
/* +2 to allow for a trailing (stripped) '\n' and null-terminator */
15051505
unsigned char buf[INQUIRY_REVISION_LEN + 2];
15061506
char *stripped = NULL;
1507-
size_t len;
1507+
ssize_t len;
15081508
ssize_t ret;
15091509

1510-
len = strlcpy(buf, page, sizeof(buf));
1511-
if (len < sizeof(buf)) {
1510+
len = strscpy(buf, page, sizeof(buf));
1511+
if (len > 0) {
15121512
/* Strip any newline added from userspace. */
15131513
stripped = strstrip(buf);
15141514
len = strlen(stripped);
15151515
}
1516-
if (len > INQUIRY_REVISION_LEN) {
1516+
if (len < 0 || len > INQUIRY_REVISION_LEN) {
15171517
pr_err("Emulated T10 Revision exceeds INQUIRY_REVISION_LEN: "
15181518
__stringify(INQUIRY_REVISION_LEN)
15191519
"\n");

0 commit comments

Comments
 (0)