linux-kernel - Re: [tip: perf/core] perf/core: Convert snprintf() to scnprintf()

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: <YyrrE8vpFSR+kdHQ@gmail.com>
Date:   Wed, 21 Sep 2022 12:44:35 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org,
        Jules Irenge <jbi.octave@...il.com>, x86@...nel.org
Subject: Re: [tip: perf/core] perf/core: Convert snprintf() to scnprintf()


* Peter Zijlstra <peterz@...radead.org> wrote:

> On Wed, Sep 21, 2022 at 08:08:55AM -0000, tip-bot2 for Jules Irenge wrote:
> > The following commit has been merged into the perf/core branch of tip:
> > 
> > Commit-ID:     678739d622ae7b75b62d550858b6bf104c43e2df
> > Gitweb:        https://git.kernel.org/tip/678739d622ae7b75b62d550858b6bf104c43e2df
> > Author:        Jules Irenge <jbi.octave@...il.com>
> > AuthorDate:    Sun, 18 Sep 2022 00:41:08 +01:00
> > Committer:     Ingo Molnar <mingo@...nel.org>
> > CommitterDate: Wed, 21 Sep 2022 10:01:20 +02:00
> > 
> > perf/core: Convert snprintf() to scnprintf()
> > 
> > Coccinelle reports a warning:
> > 
> >     WARNING: use scnprintf or sprintf
> > 
> > Adding to that, there has also been some slow migration from snprintf to scnprintf.
> > 
> > This LWN article explains the rationale for this change:
> > 
> >     https: //lwn.net/Articles/69419/
> > 
> > No change in behavior.
> > 
> > [ mingo: Improved the changelog. ]
> 
> And yet, at this point I still have no clue what's wrong with
> snprintf(). So not much improvement :/

I've added this to the changelog:

    perf/core: Convert snprintf() to scnprintf()
    
    Coccinelle reports a warning:
    
        WARNING: use scnprintf or sprintf
    
    This LWN article explains the rationale for this change:
    
        https: //lwn.net/Articles/69419/
    
    Ie. snprintf() returns what *would* be the resulting length,
    while scnprintf() returns the actual length.
    
    Adding to that, there has also been some slow migration from snprintf to scnprintf,
    here's the shift in usage in the past 3.5 years, in all fs/ files:
    
                             v5.0    v6.0-rc6
       --------------------------------------
       snprintf() uses:        63         213
       scnprintf() uses:      374         186
    
    No intended change in behavior.

I also reviewed the usage sites - this patch could in fact be a bugfix as 
we are passing back these lengths to the VFS, which lengths are arguably 
bogus in the snprintf() case, should we ever get close to the buffer limits 
to trigger output truncation - which with PAGE_SIZE-1 is very unlikely.

[ Anyway, the shift in interface usage for FS code makes it pretty clear 
  that when in doubt we should use scnprintf() for FS code. snprintf() is 
  arguably actively dangerous whenever it works differently from sprintf() 
  ... ]

Thanks,

	Ingo