netdev - Re: [PATCH v2 iproute2] ifstat: convert sprintf to snprintf

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: <20240211091802.21885973@hermes.local>
Date: Sun, 11 Feb 2024 09:18:02 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Denis Kirjanov <dkirjanov@...e.de>
Cc: Denis Kirjanov <kirjanov@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v2 iproute2] ifstat: convert sprintf to snprintf

On Sun, 11 Feb 2024 11:39:13 +0300
Denis Kirjanov <dkirjanov@...e.de> wrote:

> On 2/10/24 23:33, Stephen Hemminger wrote:
> > On Fri,  2 Feb 2024 04:35:27 -0500
> > Denis Kirjanov <kirjanov@...il.com> wrote:
> >   
> >> Use snprintf to print only valid data
> >>
> >> v2: adjust formatting
> >>
> >> Signed-off-by: Denis Kirjanov <dkirjanov@...e.de>
> >> ---  
> > 
> > Tried this but compile failed
> > 
> > ifstat.c:896:2: warning: 'snprintf' size argument is too large; destination buffer has size 107, but size argument is 108 [-Wfortify-source]
> >         snprintf(sun.sun_path + 1, sizeof(sun.sun_path), "ifstat%d", getuid());  
> 
> Right, this is addressed in the patch with scnprintf
>  

But I see no need to convert to scnprintf(). Scnprintf is about the return value
and almost nowhere in iproute2 uses the return value and those that to look at the
return value are checking for beyond buffer. Plus if you convert to scnprintf you
lose lots of the fortify and other analyzer checking.

Bottom line scnprintf() makes sense in kernel but not iproute2.