linux-kernel - Re: [PATCH] vsprintf: sanely handle NULL passed to %pe

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]

Date:   Sat, 22 Feb 2020 00:52:27 +0100
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Petr Mladek <pmladek@...e.com>, Ilya Dryomov <idryomov@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jonathan Corbet <corbet@....net>,
        Kees Cook <keescook@...omium.org>,
        "Tobin C . Harding" <me@...in.cc>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] vsprintf: sanely handle NULL passed to %pe

On 21/02/2020 14.05, Petr Mladek wrote:
> On Thu 2020-02-20 16:02:48, Ilya Dryomov wrote:

>> I would like to see it in 5.6, so that it is backported to 5.4 and 5.5.
> 
> OK, it would make sense to make the patch minimalist to make it
> easier for backporting.
> 
> 
>> Please note that I sent v2 of my patch ("[PATCH v2] vsprintf: don't
>> obfuscate NULL and error pointers"), fixing null_pointer() and adding
>> error_pointer() test cases, which conflicts with this restructure.
> 
> IMHO, v2 creates even more mess in print tests that would need
> to be fixed later.
> 
> If we agree to have a minimalist patch for backport
> then I suggest to take v1. We could clean up and update
> tests later.
> 
> Rasmus, others, is anyone against this approach (v1 first,
> tests later)?

Sorry to be that guy, but yes, I'm against changing the behavior of
vsnprintf() without at least some test(s) added to the test suite - the
lack of machine-checked documentation in the form of tests is what led
to that regression in the first place.

But I agree that there's no point adding another helper function and
muddying the test suite even more (especially as the name error_pointer
is too close to the name errptr() I chose a few months back for the %pe).

So how about

- remove the now stale test_hashed("%p", NULL); from null_pointer()
- add tests of "%p", NULL and "%p", ERR_PTR(-123) to plain()

and we save testing the "%px" case for when we figure out a good name
for a helper for that (explicit_pointer? pointer_as_hex?)

?

Rasmus