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: <Yyr8ZHxjXddYbHhj@octinomon>
Date:   Wed, 21 Sep 2022 12:58:28 +0100
From:   Jules Irenge <jbi.octave@...il.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Elana.Copperman@...ileye.com, mingo@...nel.org,
        linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org
Subject: Re: [tip: perf/core] perf/core: Convert snprintf() to scnprintf()

On Wed, Sep 21, 2022 at 10:34:35AM +0200, Peter Zijlstra 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 :/
> 
> As such I'm still very much against this patch.

Hi Peter,

Thanks for the feedback,

My bad, I am still a newbie. I will try to improve on my changelog next time.

But I have learned that the difference is as Ingo pointed out:

snprintf return the length of the buffer to be written with assumption it all fits in the destination array
while scnprintf return the actual length that fit in the destination array(eg. buf below).

This is just by precaution or safety in mind in case the PAGE - 1 is
overun.

I did some digging and came up with a code like this for the corner
case.

#define BUFSIZE 4
static int __init my_init(void)
{
        char buf[BUFSIZE];
        int x,y;
        
	x = snprintf(buf, BUFSIZE, "Linux"); // length is 5 here : return length of expected to be written when the BUFFSIZE is 4
        pr_info(" With length %d, The string is %s\n", x, buf);

        y = scnprintf(buf, BUFSIZE, "Linux"); //length is 3 : return length of what is actually written to buff
	pr_info(" With length %d, The string is %s\n", y, buf);

        return 0;
}


I appreciate any comment as I am on learning journey.

Thank you,
Jules