linux-kernel - Re: [PATCH] printk.c: removed unnecessary code

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: <20161130102231.GE24060@pathway.suse.cz>
Date:   Wed, 30 Nov 2016 11:22:31 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Sebastian Duda <sebastian.duda@....de>, linux-kernel@...cs.fau.de,
        Tobias Baumeister <tobias.baumeister@....de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Borislav Petkov <bp@...e.de>, Tejun Heo <tj@...nel.org>,
        Thierry Reding <treding@...dia.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] printk.c: removed unnecessary code

On Wed 2016-11-30 10:14:28, Michal Hocko wrote:
> [Resending with the full CC list as my email client has clobbered it in
> the previous attempt for some reason]
> 
> On Tue 29-11-16 16:19:01, Sebastian Duda wrote:
> > snprintf((char *) ?, 0, ...); always returns Zero and doesn't change the data.
> > Thus the execution of
> > 	snprintf(NULL, 0, "[%5lu.000000] ", (unsigned long)ts);
> > has no effect on program.
> > The substitution with 0 increases the readability of the code.
> 
> Are you sure this is correct. As per vsnprintf documentation:
> "
>  * The return value is the number of characters which would
>  * be generated for the given input, excluding the trailing
>  * '\0', as per ISO C99.
> "
> 
> this should just work as 35dac27cedd1 ("printk: fix incorrect length
> from print_time() when seconds > 99999") intended.
> 
> I haven't checked the implementation though so I might be wrong here.

print_time() actually is not used by printk() directly. Therefore
the above reasoning does not apply.

They key here is to look when the buf might be NULL. It is when
we try to get an idea how much space will be needed for the message
when it is printed to some output device, e.g. console or syslog.
For example, there is the following chain of calls:

  syslog_print_all()
     msg_print_text(msg, prev, true, NULL, 0)
       print_prefix(msg, syslog, NULL)
         len += print_time(msg->ts_nsec, buf ? buf + len : NULL)

It means that we really need to know the length of the printed
string even when we do not write it to a buffer for the moment.

By other words, the patch would break the functionality.

> > 
> > Signed-off-by: Sebastian Duda <sebastian.duda@....de>
> > Signed-off-by: Tobias Baumeister <tobias.baumeister@....de>

Nacked-by: Petr Mladek <pmladek@...e.com>

Best Regards,
Petr