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] [thread-next>] [day] [month] [year] [list]

Message-ID: <e53a2d46-fd6d-d0cc-8b78-205c5bd6784b@pensando.io>
Date:   Fri, 11 Jun 2021 09:28:05 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     Lijun Pan <lijunp213@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] ibmvnic: fix kernel build warning in strncpy

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