linux-hardening - Re: [PATCH] vsprintf: fix possible NULL pointer deref in vsnprintf()

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: <CAHk-=wgCTaDeS9gPf=57rBMCOw6MGmzCHywX=en05=pZaCnjuw@mail.gmail.com>
Date:   Fri, 6 Jan 2023 11:07:33 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Shtylyov <s.shtylyov@....ru>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Kees Cook <keescook@...omium.org>,
        linux-hardening@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vsprintf: fix possible NULL pointer deref in vsnprintf()

On Fri, Jan 6, 2023 at 7:49 AM Petr Mladek <pmladek@...e.com> wrote:
>
> Adding Kees, Linus, and linux-hardening into Cc. It seems that
> __vsnprintf_internal() does not do this in glibc. I wonder if
> there is a good reason for it.

I do not think that patch is valid.

And I don't like Steven Rostedt's suggestion either.

I think that code *should* take a NULL pointer dereference, exactly
the same way "strlen()" would do if you pass it a NULL pointer and
claim there is room there.

No silly checks for invalid cases.

There's any number of invalid things you can do in the kernel, and we
don't check for those either.

I don't particularly like the "pass NULL to sprintf()" thing at all,
but *if* somebody does, they had better just pass a zero size too.

And doing

        git grep 'sn*printf(NULL'

seems to show that all current users do exactly that.

If somebody wants to check for this, make it a coccinelle script or
something. Not a runtime check for invalid use cases.

               Linus