target/core: replace strncpy with strscpy

[PATCH] target/core: replace strncpy with strscpy

Posted by Pranav Tyagi 3 months ago

strncpy() is deprecated and its use is discouraged. Replace strncpy()
with safer strscpy() as the p_buf buffer should be NUL-terminated, since
it holds human readable debug output strings.

Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
---
 drivers/target/target_core_transport.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 0a76bdfe5528..9c255ed21789 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1112,7 +1112,7 @@ void transport_dump_vpd_proto_id(
 	}
 
 	if (p_buf)
-		strncpy(p_buf, buf, p_buf_len);
+		strscpy(p_buf, buf, p_buf_len);
 	else
 		pr_debug("%s", buf);
 }
@@ -1162,7 +1162,7 @@ int transport_dump_vpd_assoc(
 	}
 
 	if (p_buf)
-		strncpy(p_buf, buf, p_buf_len);
+		strscpy(p_buf, buf, p_buf_len);
 	else
 		pr_debug("%s", buf);
 
@@ -1222,7 +1222,7 @@ int transport_dump_vpd_ident_type(
 	if (p_buf) {
 		if (p_buf_len < strlen(buf)+1)
 			return -EINVAL;
-		strncpy(p_buf, buf, p_buf_len);
+		strscpy(p_buf, buf, p_buf_len);
 	} else {
 		pr_debug("%s", buf);
 	}
@@ -1276,7 +1276,7 @@ int transport_dump_vpd_ident(
 	}
 
 	if (p_buf)
-		strncpy(p_buf, buf, p_buf_len);
+		strscpy(p_buf, buf, p_buf_len);
 	else
 		pr_debug("%s", buf);
 
-- 
2.49.0

Re: [PATCH] target/core: replace strncpy with strscpy

Posted by Martin K. Petersen 2 months, 2 weeks ago

Pranav,

> strncpy() is deprecated and its use is discouraged. Replace strncpy()
> with safer strscpy() as the p_buf buffer should be NUL-terminated,
> since it holds human readable debug output strings.

If you must do this, please change all the similar occurrences of
strncpy() in that file instead of just one of them.

However, given the fixed size of the buffer and the length of all the
defined static strings, what is the actual problem you are fixing?

-- 
Martin K. Petersen

Re: [PATCH] target/core: replace strncpy with strscpy

Posted by Pranav Tyagi 2 months, 2 weeks ago

On Fri, Jul 25, 2025 at 7:22 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Pranav,
>
> > strncpy() is deprecated and its use is discouraged. Replace strncpy()
> > with safer strscpy() as the p_buf buffer should be NUL-terminated,
> > since it holds human readable debug output strings.
>
> If you must do this, please change all the similar occurrences of
> strncpy() in that file instead of just one of them.
>
> However, given the fixed size of the buffer and the length of all the
> defined static strings, what is the actual problem you are fixing?
>
> --
> Martin K. Petersen

Hi Martin,

As far as I looked, I could only find the following 4 instances of
strncpy() for the file target_core_transport.c:

target_core_transport.c:1115:           strncpy(p_buf, buf, p_buf_len);
target_core_transport.c:1165:           strncpy(p_buf, buf, p_buf_len);
target_core_transport.c:1225:           strncpy(p_buf, buf, p_buf_len);
target_core_transport.c:1279:           strncpy(p_buf, buf, p_buf_len);

And I have changed all of them in my patch. Kindly point me out to
other instances, if I am missing any.

Also, I intended this to be a cleanup patch for the deprecated
strncpy() function and wanted to replace it with strscpy()
which is encouraged. No functional changes were intended.

Regards
Pranav Tyagi

Re: [PATCH] target/core: replace strncpy with strscpy

Posted by Martin K. Petersen 2 months, 1 week ago

Hi Pranav!

> As far as I looked, I could only find the following 4 instances of
> strncpy() for the file target_core_transport.c:
>
> target_core_transport.c:1115:           strncpy(p_buf, buf, p_buf_len);
> target_core_transport.c:1165:           strncpy(p_buf, buf, p_buf_len);
> target_core_transport.c:1225:           strncpy(p_buf, buf, p_buf_len);
> target_core_transport.c:1279:           strncpy(p_buf, buf, p_buf_len);
>
> And I have changed all of them in my patch. Kindly point me out to
> other instances, if I am missing any.

Sorry, I guess I didn't read far enough. I was focused on the VPD
identifier dump function and whether it could overrun the static buffer.

> Also, I intended this to be a cleanup patch for the deprecated
> strncpy() function and wanted to replace it with strscpy() which is
> encouraged. No functional changes were intended.

In our experience cleanup patches come with a very high risk of
introducing regressions. Regressions in the I/O stack could potentially
lead to issues such as systems failing to boot or people losing their
data. So we generally only merge patches if it can be demonstrated that
they fix an actual problem.

-- 
Martin K. Petersen

Re: [PATCH] target/core: replace strncpy with strscpy

Posted by Pranav Tyagi 2 months, 1 week ago

On Tue, Jul 29, 2025 at 8:39 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>
>
> Hi Pranav!
>
> > As far as I looked, I could only find the following 4 instances of
> > strncpy() for the file target_core_transport.c:
> >
> > target_core_transport.c:1115:           strncpy(p_buf, buf, p_buf_len);
> > target_core_transport.c:1165:           strncpy(p_buf, buf, p_buf_len);
> > target_core_transport.c:1225:           strncpy(p_buf, buf, p_buf_len);
> > target_core_transport.c:1279:           strncpy(p_buf, buf, p_buf_len);
> >
> > And I have changed all of them in my patch. Kindly point me out to
> > other instances, if I am missing any.
>
> Sorry, I guess I didn't read far enough. I was focused on the VPD
> identifier dump function and whether it could overrun the static buffer.
>
> > Also, I intended this to be a cleanup patch for the deprecated
> > strncpy() function and wanted to replace it with strscpy() which is
> > encouraged. No functional changes were intended.
>
> In our experience cleanup patches come with a very high risk of
> introducing regressions. Regressions in the I/O stack could potentially
> lead to issues such as systems failing to boot or people losing their
> data. So we generally only merge patches if it can be demonstrated that
> they fix an actual problem.
>
> --
> Martin K. Petersen

Hi Martin,

I did not know about the risks involved with cleanup patches for the
I/O stack. I understand that this could lead to regressions and it is best
to drop this patch. Thank you for reviewing it.

Regards
Pranav Tyagi

Re: [PATCH] target/core: replace strncpy with strscpy

Posted by Pranav Tyagi 2 months, 4 weeks ago

On Sun, Jul 6, 2025 at 2:55 PM Pranav Tyagi <pranav.tyagi03@gmail.com> wrote:
>
> strncpy() is deprecated and its use is discouraged. Replace strncpy()
> with safer strscpy() as the p_buf buffer should be NUL-terminated, since
> it holds human readable debug output strings.
>
> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> ---
>  drivers/target/target_core_transport.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 0a76bdfe5528..9c255ed21789 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -1112,7 +1112,7 @@ void transport_dump_vpd_proto_id(
>         }
>
>         if (p_buf)
> -               strncpy(p_buf, buf, p_buf_len);
> +               strscpy(p_buf, buf, p_buf_len);
>         else
>                 pr_debug("%s", buf);
>  }
> @@ -1162,7 +1162,7 @@ int transport_dump_vpd_assoc(
>         }
>
>         if (p_buf)
> -               strncpy(p_buf, buf, p_buf_len);
> +               strscpy(p_buf, buf, p_buf_len);
>         else
>                 pr_debug("%s", buf);
>
> @@ -1222,7 +1222,7 @@ int transport_dump_vpd_ident_type(
>         if (p_buf) {
>                 if (p_buf_len < strlen(buf)+1)
>                         return -EINVAL;
> -               strncpy(p_buf, buf, p_buf_len);
> +               strscpy(p_buf, buf, p_buf_len);
>         } else {
>                 pr_debug("%s", buf);
>         }
> @@ -1276,7 +1276,7 @@ int transport_dump_vpd_ident(
>         }
>
>         if (p_buf)
> -               strncpy(p_buf, buf, p_buf_len);
> +               strscpy(p_buf, buf, p_buf_len);
>         else
>                 pr_debug("%s", buf);
>
> --
> 2.49.0
>
Hi,

This is a gentle follow-up on this patch. I would like to
know if there is any update on its state.

Regards
Pranav Tyagi