linux-kernel - Re: [PATCH v4 3/9] vsprintf: Do not check address of well-known strings

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 Apr 2018 11:15:43 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Tobin C . Harding" <me@...in.cc>, Joe Perches <joe@...ches.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.cz>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/9] vsprintf: Do not check address of well-known
 strings

On Thu 2018-04-05 15:30:51, Rasmus Villemoes wrote:
> On 2018-04-04 10:58, Petr Mladek wrote:
> > We are going to check the address using probe_kernel_address(). It will
> > be more expensive and it does not make sense for well known address.
> > 
> > This patch splits the string() function. The variant without the check
> > is then used on locations that handle string constants or strings defined
> > as local variables.
> > 
> > This patch does not change the existing behavior.
> 
> Please leave string() alone, except for moving the < PAGE_SIZE check to
> a new helper checked_string (feel free to find a better name), and use
> checked_string for handling %s and possibly the few other cases where
> we're passing a user-supplied pointer. That avoids cluttering the entire
> file with double-underscore calls, and e.g. in the %pO case, it's easier
> to understand why one uses two different *string() helpers if the name
> of one somehow conveys how it is different from the other.

I understand your reasoning. I thought about exactly this as well.
My problem is that string() will then be unsafe. It might be dangerous
when porting patches.

This is why I wanted a different name for the variant without the
check. But I was not able to come up with anything short and clear
at the same time.

Is _string() really that bad? I think that it is a rather common
practice to use _func() for functions that are less safe than func()
variants. People should use _func() variants with care and this is
what we want here.

In addition, it is an internal API. IMHO, only few people do changes
there. They will get used to it quickly. Which is not true for people
that might need to port patches.

Best Regards,
Petr