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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bf41b970-d2bd-9e44-aaaa-0820cf54867b@pensando.io>
Date:   Fri, 11 Jun 2021 11:59:12 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     Lijun Pan <lijunp213@...il.com>
Cc:     Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] ibmvnic: fix kernel build warning in strncpy

On 6/11/21 11:36 AM, Lijun Pan wrote:
> On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson <snelson@...sando.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@...il.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