[PATCH net-next] ibmvnic: fix kernel build warning in strncpy

drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’:
drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
 4393 |   strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Lijun Pan <redacted>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 497f1a7da70b..2675b2301ed7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c

@@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq,
 
 complete:
 	if (adapter->fw_version[0] == '\0')
-		strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
+		memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
 	complete(&adapter->fw_done);
 }
 

-- 
2.23.0

On 6/11/21 9:05 AM, Lijun Pan wrote:

This doesn't fix the real problem.  The error message is saying that 
there is no string terminating '\0' byte getting set after the "N/A" 
string, meaning that there could be garbage in the buffer after the 
string that could allow for surprising and bad things to happen when 
that string is used later, including buffer overruns that can cause 
stack smash or other memory munging.

Better would be to use strlcpy() with a limiter of 
sizeof(adapter->fw_version).

sln

On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson [off-list ref] wrote:

On 6/11/21 9:05 AM, Lijun Pan wrote:

quoted

drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’:
drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
  4393 |   strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Lijun Pan <redacted>
---
  drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 497f1a7da70b..2675b2301ed7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c

@@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq,

  complete:
      if (adapter->fw_version[0] == '\0')
-             strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
+             memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
      complete(&adapter->fw_done);
  }

This doesn't fix the real problem.  The error message is saying that
there is no string terminating '\0' byte getting set after the "N/A"
string, meaning that there could be garbage in the buffer after the
string that could allow for surprising and bad things to happen when
that string is used later, including buffer overruns that can cause
stack smash or other memory munging.

Better would be to use strlcpy() with a limiter of
sizeof(adapter->fw_version).

sln

Thanks for the tip. I looked up both strscpy and strlcpy. It seems nowadays
strscpy is preferred.

On 6/11/21 11:36 AM, Lijun Pan wrote:

On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson [off-list ref] wrote:

quoted

On 6/11/21 9:05 AM, Lijun Pan wrote:

quoted

drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’:
drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
   4393 |   strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Lijun Pan <redacted>
---
   drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 497f1a7da70b..2675b2301ed7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c

@@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq,

   complete:
       if (adapter->fw_version[0] == '\0')
-             strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
+             memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char));
       complete(&adapter->fw_done);
   }

This doesn't fix the real problem.  The error message is saying that
there is no string terminating '\0' byte getting set after the "N/A"
string, meaning that there could be garbage in the buffer after the
string that could allow for surprising and bad things to happen when
that string is used later, including buffer overruns that can cause
stack smash or other memory munging.

Better would be to use strlcpy() with a limiter of
sizeof(adapter->fw_version).

sln

Thanks for the tip. I looked up both strscpy and strlcpy. It seems nowadays
strscpy is preferred.

Sure, that works too.
sln