linux-kernel - Re: [PATCH] vsprintf: Make "null" pointer dereference more robust

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: <CA+55aFwT-Vd+sVdAVemUjUvkNYFiLyVMHZxv9hF6KzRM7i4Yrg@mail.gmail.com>
Date:   Fri, 9 Mar 2018 11:05:17 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        "Tobin C . Harding" <me@...in.cc>, Joe Perches <joe@...ches.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.cz>
Subject: Re: [PATCH] vsprintf: Make "null" pointer dereference more robust

On Fri, Mar 9, 2018 at 7:01 AM, Petr Mladek <pmladek@...e.com> wrote:
> Also it makes the handling unified. We print:
>
>    + (null) when pure NULL pointer is dereferenced
>    + (efault) when an invalid address is dereferenced
>    + pointer address otherwise

This is still fundamentally completely wrong.

It never prints "pointer address", and if it were to do that, it would be wrong.

It should never ever trigger for an address operation, only for the
"we will get _data_ from the ponter".

The strchr thing is also completely broken, and in a very subtle way.
"strchr(string, 0)" is special, and the Open Group states

  "The terminating null byte is considered to be part of the string"

so a NUL character will *always* return success, which is actually
completely wrong for this case, because now it does that whole crazy
<efault> thing for %p that it shouldn't do.

Not that I actually verified that our strchr() follows the actual
rules anyway - I personally consider "strchr(string, 0)" to not really
be "special", but be a bug. Either way, the comment is wrong, but the
code is also wrong.

                 Linus