netdev - Re: [PATCH net-next] drivers: net: sky2: Fix -Wstringop-truncation with W=1

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: <20201102160106.29edcc11@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Mon, 2 Nov 2020 16:01:06 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] drivers: net: sky2: Fix -Wstringop-truncation
 with W=1

On Sat, 31 Oct 2020 18:40:28 +0100 Andrew Lunn wrote:
> In function ‘strncpy’,
>     inlined from ‘sky2_name’ at drivers/net/ethernet/marvell/sky2.c:4903:3,
>     inlined from ‘sky2_probe’ at drivers/net/ethernet/marvell/sky2.c:5049:2:
> ./include/linux/string.h:297:30: warning: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Wstringop-truncation]
> 
> None of the device names are 16 characters long, so it was never an
> issue, but reduce the length of the buffer size by one to avoid the
> warning.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>
> ---
>  drivers/net/ethernet/marvell/sky2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
> index 25981a7a43b5..35b0ec5afe13 100644
> --- a/drivers/net/ethernet/marvell/sky2.c
> +++ b/drivers/net/ethernet/marvell/sky2.c
> @@ -4900,7 +4900,7 @@ static const char *sky2_name(u8 chipid, char *buf, int sz)
>  	};
>  
>  	if (chipid >= CHIP_ID_YUKON_XL && chipid <= CHIP_ID_YUKON_OP_2)
> -		strncpy(buf, name[chipid - CHIP_ID_YUKON_XL], sz);
> +		strncpy(buf, name[chipid - CHIP_ID_YUKON_XL], sz - 1);

Hm. This irks the eye a little. AFAIK the idiomatic code would be:

	strncpy(buf, name..., sz - 1);
	buf[sz - 1] = '\0';

Perhaps it's easier to convert to strscpy()/strscpy_pad()?

>  	else
>  		snprintf(buf, sz, "(chip %#x)", chipid);
>  	return buf;