diff mbox series

[v2] mpi3mr: Fix W=1 compilation warnings

Message ID 20210629141110.3098-1-sreekanth.reddy@broadcom.com
State New
Headers show
Series [v2] mpi3mr: Fix W=1 compilation warnings | expand

Commit Message

Sreekanth Reddy June 29, 2021, 2:11 p.m. UTC
Fix for below W=1 compilation warning,
'strncpy' output may be truncated copying 16 bytes
 from a string of length 64

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Sreekanth Reddy July 4, 2021, 5:09 p.m. UTC | #1
On Wed, Jun 30, 2021 at 2:06 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>

>

> Sreekanth,

>

> > -     strncpy(drv_info->os_name, utsname()->sysname, sizeof(drv_info->os_name));

> > +     strscpy(drv_info->os_name, utsname()->sysname, sizeof(drv_info->os_name));

> >       drv_info->os_name[sizeof(drv_info->os_name) - 1] = 0;

>

> strscpy() terminates the string.


I verified strscpy() is not adding any null terminator when source
string length is greater than destination buffer size. And we need the
string to be null terminated. or Can I replace it as
strscpy(drv_info->os_name, utsname()->sysname, sizeof(drv_info->os_name) -1);

>

> > -     strncpy(drv_info->os_version, utsname()->release, sizeof(drv_info->os_version));

> > +     strscpy(drv_info->os_version, utsname()->release, sizeof(drv_info->os_version));

> >       drv_info->os_version[sizeof(drv_info->os_version) - 1] = 0;

>

> Same here.

>

> >       strncpy(drv_info->driver_name, MPI3MR_DRIVER_NAME, sizeof(drv_info->driver_name));

> >       strncpy(drv_info->driver_version, MPI3MR_DRIVER_VERSION, sizeof(drv_info->driver_version));

>

> Please convert the remaining strncpy() calls as well.


Sure. I will replace strncpy() with strscpy() in the next update patch.

Thanks,
Sreekanth

>

> Thanks!

>

> --

> Martin K. Petersen      Oracle Linux Engineering
Martin K. Petersen July 6, 2021, 6:39 p.m. UTC | #2
Sreekanth,

> I verified strscpy() is not adding any null terminator when source

> string length is greater than destination buffer size.


That's odd. The point of strscpy() is that it guarantees termination:

---8<---
/**
 * strscpy - Copy a C-string into a sized buffer
 * @dest: Where to copy the string to
 * @src: Where to copy the string from
 * @count: Size of destination buffer
 *
 * Copy the string, or as much of it as fits, into the dest buffer.  The
 * behavior is undefined if the string buffers overlap.  The destination
 * buffer is always NUL terminated, unless it's zero-sized.
---8<---

I tested it and it works fine for me.

-- 
Martin K. Petersen	Oracle Linux Engineering
Sreekanth Reddy July 7, 2021, 5:53 a.m. UTC | #3
On Wed, Jul 7, 2021 at 12:09 AM Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>

>

> Sreekanth,

>

> > I verified strscpy() is not adding any null terminator when source

> > string length is greater than destination buffer size.

>

> That's odd. The point of strscpy() is that it guarantees termination:

>

> ---8<---

> /**

>  * strscpy - Copy a C-string into a sized buffer

>  * @dest: Where to copy the string to

>  * @src: Where to copy the string from

>  * @count: Size of destination buffer

>  *

>  * Copy the string, or as much of it as fits, into the dest buffer.  The

>  * behavior is undefined if the string buffers overlap.  The destination

>  * buffer is always NUL terminated, unless it's zero-sized.

> ---8<---

>

> I tested it and it works fine for me.


Ok, my bad. I made a small mistake while verifying it. Now I rectified
it and saw that strscpy() is adding a NULL character at the end.

Thanks,
Sreekanth

>

> --

> Martin K. Petersen      Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 9eceafca59bc..ede2bd0cf8d4 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -2608,9 +2608,9 @@  static int mpi3mr_issue_iocinit(struct mpi3mr_ioc *mrioc)
 	}
 	drv_info->information_length = cpu_to_le32(data_len);
 	strncpy(drv_info->driver_signature, "Broadcom", sizeof(drv_info->driver_signature));
-	strncpy(drv_info->os_name, utsname()->sysname, sizeof(drv_info->os_name));
+	strscpy(drv_info->os_name, utsname()->sysname, sizeof(drv_info->os_name));
 	drv_info->os_name[sizeof(drv_info->os_name) - 1] = 0;
-	strncpy(drv_info->os_version, utsname()->release, sizeof(drv_info->os_version));
+	strscpy(drv_info->os_version, utsname()->release, sizeof(drv_info->os_version));
 	drv_info->os_version[sizeof(drv_info->os_version) - 1] = 0;
 	strncpy(drv_info->driver_name, MPI3MR_DRIVER_NAME, sizeof(drv_info->driver_name));
 	strncpy(drv_info->driver_version, MPI3MR_DRIVER_VERSION, sizeof(drv_info->driver_version));