drivers/target/target_core_configfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Signed-off-by: goralbaris <goralbaris@gmail.com>
---
drivers/target/target_core_configfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, read_bytes);
+ strscpy(db_root, db_root_stage, read_bytes);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+ strscpy(db_root, db_root_stage, DB_ROOT_LEN);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
}
--
2.34.1
On Wed, Apr 02, 2025 at 08:25:04PM +0300, goralbaris wrote: > Signed-off-by: goralbaris <goralbaris@gmail.com> I think something went wrong with your subject line :( Also, don't use your email alias for signed-off-by please. thanks, greg k-h
Description:
The strncpy() function is actively dangerous to use since it may not NULL-terminate the destination string,
resulting in potential memory content exposures, unbounded reads, or crashes.
Link: [1] https://github.com/KSPP/linux/issues/90
Changes from v2:
-Description added
-User Name corrected
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
drivers/target/target_core_configfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, read_bytes);
+ strscpy(db_root, db_root_stage, read_bytes);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+ strscpy(db_root, db_root_stage, DB_ROOT_LEN);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
}
--
2.34.1
Description:
The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string,resulting in potential memory
content exposures, unbounded reads, or crashes.
Link:https://github.com/KSPP/linux/issues/90
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
Changes from v3:
-Description added
-User name corrected
-formatting issues.
drivers/target/target_core_configfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, read_bytes);
+ strscpy(db_root, db_root_stage, read_bytes);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+ strscpy(db_root, db_root_stage, DB_ROOT_LEN);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
}
--
2.34.1
Hello, two small things: On Wed Apr 2, 2025 at 10:45 PM CEST, Baris Can Goral wrote: > Description: You can remove this "description" tag. Also, it's better to add a prefix to the subject, for example: "scsi: target: transform strncpy into strscpy" Maurizio
The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string,resulting in potential memory
content exposures, unbounded reads, or crashes.
Link:https://github.com/KSPP/linux/issues/90
Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
Changes from v4:
-Description added
-User name corrected
-formatting issues.
-commit name changed
drivers/target/target_core_configfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c40217f44b1b..5c0b74e76be2 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, read_bytes);
+ strscpy(db_root, db_root_stage, read_bytes);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+ strscpy(db_root, db_root_stage, DB_ROOT_LEN);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
}
--
2.34.1
On Sat Apr 5, 2025 at 4:36 PM CEST, Baris Can Goral wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string,resulting in potential memory
> content exposures, unbounded reads, or crashes.
>
> Link:https://github.com/KSPP/linux/issues/90
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
> Changes from v4:
> -Description added
> -User name corrected
> -formatting issues.
> -commit name changed
> drivers/target/target_core_configfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index c40217f44b1b..5c0b74e76be2 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> }
> filp_close(fp, NULL);
>
> - strncpy(db_root, db_root_stage, read_bytes);
> + strscpy(db_root, db_root_stage, read_bytes);
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
>
> r = read_bytes;
> @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
> }
> filp_close(fp, NULL);
>
> - strncpy(db_root, db_root_stage, DB_ROOT_LEN);
> + strscpy(db_root, db_root_stage, DB_ROOT_LEN);
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> }
>
This patch doesn't apply anymore.
strncpy() has already been replaced with strscpy()
in version 6.14-rc2.
Maurizio
Hi Maurizio, Unfourtunately, in version 6.14-rc2 (and also v6.14) strncpy is still there. Best Regards, Baris
On Mon Apr 7, 2025 at 7:48 PM CEST, Baris Can Goral wrote: > Hi Maurizio, > > Unfourtunately, in version 6.14-rc2 (and also v6.14) strncpy is still there. Sorry, my mistake. commit dfb7df1ddb29c89662e84b2c82c1ff7943358ae0 that replaces strncpy() with strscpy() has been merged in kernel version 6.15-rc1 Maurizio
On Sat, 5 Apr 2025 17:36:47 +0300
Baris Can Goral <goralbaris@gmail.com> wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string,resulting in potential memory
> content exposures, unbounded reads, or crashes.
>
> Link:https://github.com/KSPP/linux/issues/90
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
> Changes from v4:
> -Description added
> -User name corrected
> -formatting issues.
> -commit name changed
> drivers/target/target_core_configfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index c40217f44b1b..5c0b74e76be2 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
> }
> filp_close(fp, NULL);
>
> - strncpy(db_root, db_root_stage, read_bytes);
> + strscpy(db_root, db_root_stage, read_bytes);
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
That code is broken, it reads:
read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
if (!read_bytes)
goto unlock;
if (db_root_stage[read_bytes - 1] == '\n')
db_root_stage[read_bytes - 1] = '\0';
/* validate new db root before accepting it */
fp = filp_open(db_root_stage, O_RDONLY, 0);
if (IS_ERR(fp)) {
pr_err("db_root: cannot open: %s\n", db_root_stage);
goto unlock;
}
if (!S_ISDIR(file_inode(fp)->i_mode)) {
filp_close(fp, NULL);
pr_err("db_root: not a directory: %s\n", db_root_stage);
goto unlock;
}
filp_close(fp, NULL);
strncpy(db_root, db_root_stage, read_bytes);
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
unlock:
mutex_unlock(&target_devices_lock);
return r;
'Really nasty (tm)' things happen if 'page' is too long.
David
>
> r = read_bytes;
> @@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
> }
> filp_close(fp, NULL);
>
> - strncpy(db_root, db_root_stage, DB_ROOT_LEN);
> + strscpy(db_root, db_root_stage, DB_ROOT_LEN);
> pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
> }
>
© 2016 - 2026 Red Hat, Inc.