netdev - RE: [PATCH 1/2] staging: rtl8192e: Fix compiler warning about 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: <f1dfc983495c41a7b9d048419aeebd4d@AcuMS.aculab.com>
Date:   Tue, 21 Aug 2018 13:41:59 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Larry Finger' <Larry.Finger@...inger.net>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>
Subject: RE: [PATCH 1/2] staging: rtl8192e: Fix compiler warning about strncpy

From: Larry Finger
> Sent: 20 August 2018 18:51
> When strncpy() is called with source and destination strings the same
> length, gcc 8 warns that there may be an unterminated string. Using
> strlcpy() rather than strncpy() forces a null at the end and quiets the
> warning.
> 
> Signed-off-by: Larry Finger <Larry.Finger@...inger.net>
> ---
>  drivers/staging/rtl8192e/rtllib_softmac.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c
> index 919231fec09c..95a8390cb7ac 100644
> --- a/drivers/staging/rtl8192e/rtllib_softmac.c
> +++ b/drivers/staging/rtl8192e/rtllib_softmac.c
> @@ -1684,14 +1684,14 @@ inline void rtllib_softmac_new_net(struct rtllib_device *ieee,
>  			 * essid provided by the user.
>  			 */
>  			if (!ssidbroad) {
> -				strncpy(tmp_ssid, ieee->current_network.ssid,
> +				strlcpy(tmp_ssid, ieee->current_network.ssid,
>  					IW_ESSID_MAX_SIZE);
>  				tmp_ssid_len = ieee->current_network.ssid_len;

If there is a length, why not use it?
Depending on where the data came from the length might need validating.
Depending on how tmp_ssid is used it might need zero filling.

>  			}
>  			memcpy(&ieee->current_network, net,
>  			       sizeof(struct rtllib_network));

Gah - should be sizeof(ieee->current_network).
Or better still a structure assignment.

>  			if (!ssidbroad) {
> -				strncpy(ieee->current_network.ssid, tmp_ssid,
> +				strlcpy(ieee->current_network.ssid, tmp_ssid,
>  					IW_ESSID_MAX_SIZE);
>  				ieee->current_network.ssid_len = tmp_ssid_len;

Hmmm... this looks like it is restoring the fields.
So why not have a temporary ssid buffer that is the size of the
actual buffer and user memcpy().

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)