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

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

[Lijun Pan]

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 <lijunp213@gmail.com>
---
 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

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

[Shannon Nelson]

On 6/11/21 9:05 AM, Lijun Pan wrote:
> 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 <lijunp213@gmail.com>
> ---
>   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

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

[Lijun Pan]

On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson <snelson@pensando.io> wrote:
>
> On 6/11/21 9:05 AM, Lijun Pan wrote:
> > 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 <lijunp213@gmail.com>
> > ---
> >   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.

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

[Shannon Nelson]

On 6/11/21 11:36 AM, Lijun Pan wrote:
> On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson <snelson@pensando.io> wrote:
>> On 6/11/21 9:05 AM, Lijun Pan wrote:
>>> 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 <lijunp213@gmail.com>
>>> ---
>>>    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