[PATCH] staging: qlge: Replace strncpy with strscpy

[PATCH] staging: qlge: Replace strncpy with strscpy

[Ricardo Lopes]

Avoid read overflows and other misbehavior due to missing termination.

Reported by checkpatch:

WARNING: Prefer strscpy, strscpy_pad, or __nonstring over strncpy

Signed-off-by: Ricardo Lopes <ricardoapl.dev@gmail.com>
---
 drivers/staging/qlge/qlge_dbg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index c7e865f51..5f08a8492 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -696,7 +696,7 @@ static void qlge_build_coredump_seg_header(struct mpi_coredump_segment_header *s
 	seg_hdr->cookie = MPI_COREDUMP_COOKIE;
 	seg_hdr->seg_num = seg_number;
 	seg_hdr->seg_size = seg_size;
-	strncpy(seg_hdr->description, desc, (sizeof(seg_hdr->description)) - 1);
+	strscpy(seg_hdr->description, desc, sizeof(seg_hdr->description));
 }
 
 /*
@@ -737,7 +737,7 @@ int qlge_core_dump(struct qlge_adapter *qdev, struct qlge_mpi_coredump *mpi_core
 		sizeof(struct mpi_coredump_global_header);
 	mpi_coredump->mpi_global_header.image_size =
 		sizeof(struct qlge_mpi_coredump);
-	strncpy(mpi_coredump->mpi_global_header.id_string, "MPI Coredump",
+	strscpy(mpi_coredump->mpi_global_header.id_string, "MPI Coredump",
 		sizeof(mpi_coredump->mpi_global_header.id_string));
 
 	/* Get generic NIC reg dump */
@@ -1225,7 +1225,7 @@ static void qlge_gen_reg_dump(struct qlge_adapter *qdev,
 		sizeof(struct mpi_coredump_global_header);
 	mpi_coredump->mpi_global_header.image_size =
 		sizeof(struct qlge_reg_dump);
-	strncpy(mpi_coredump->mpi_global_header.id_string, "MPI Coredump",
+	strscpy(mpi_coredump->mpi_global_header.id_string, "MPI Coredump",
 		sizeof(mpi_coredump->mpi_global_header.id_string));
 
 	/* segment 16 */
-- 
2.41.0

Re: [PATCH] staging: qlge: Replace strncpy with strscpy

[Dan Carpenter]

On Thu, Oct 05, 2023 at 08:14:55PM +0100, Ricardo Lopes wrote:
> Avoid read overflows and other misbehavior due to missing termination.
> 

There aren't any read overflows in the current code.

> Reported by checkpatch:
> 
> WARNING: Prefer strscpy, strscpy_pad, or __nonstring over strncpy

But making checkpatch happy is good and the patch is fine.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

Re: [PATCH] staging: qlge: Replace strncpy with strscpy

[Dan Carpenter]

On Fri, Oct 06, 2023 at 09:42:41AM +0300, Dan Carpenter wrote:
> On Thu, Oct 05, 2023 at 08:14:55PM +0100, Ricardo Lopes wrote:
> > Avoid read overflows and other misbehavior due to missing termination.
> > 
> 
> There aren't any read overflows in the current code.
> 

So when you're reviewing these to look for read overflows, a string
literal isn't going to overflow.  So that makes the last two obvious.
But for the first one you have to review the caller qlge_gen_reg_dump()
and the last parameter passed to qlge_build_coredump_seg_header() is
always a string literal so that's obvious too.

It's not really that much work to check for this.

regards,
dan carpenter

Re: [PATCH] staging: qlge: Replace strncpy with strscpy

[Greg KH]

On Thu, Oct 05, 2023 at 08:14:55PM +0100, Ricardo Lopes wrote:
> Avoid read overflows and other misbehavior due to missing termination.

As Dan said, this is not possible here.  Can you redo the changelog text
and resend it so that people aren't scared by the text?

thanks,

greg k-h

Re: [PATCH] staging: qlge: Replace strncpy with strscpy

[Ricardo Lopes]

On Fri, Oct 06, 2023 at 03:32:28PM +0200, Greg KH wrote:
> On Thu, Oct 05, 2023 at 08:14:55PM +0100, Ricardo Lopes wrote:
> > Avoid read overflows and other misbehavior due to missing termination.
> 
> As Dan said, this is not possible here.  Can you redo the changelog text
> and resend it so that people aren't scared by the text?

Yes, I suppose removing that sentence and keeping the checkpatch.pl
warning output would do?

Thank you,

Ricardo Lopes

Re: [PATCH] staging: qlge: Replace strncpy with strscpy

[Dan Carpenter]

On Fri, Oct 06, 2023 at 04:30:21PM +0100, Ricardo Lopes wrote:
> On Fri, Oct 06, 2023 at 03:32:28PM +0200, Greg KH wrote:
> > On Thu, Oct 05, 2023 at 08:14:55PM +0100, Ricardo Lopes wrote:
> > > Avoid read overflows and other misbehavior due to missing termination.
> > 
> > As Dan said, this is not possible here.  Can you redo the changelog text
> > and resend it so that people aren't scared by the text?
> 
> Yes, I suppose removing that sentence and keeping the checkpatch.pl
> warning output would do?

Yes.

https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

regards,
dan carpenter