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:   Thu, 5 Apr 2018 16:34:12 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        "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>,
        linux-kernel@...r.kernel.org, Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v4 2/9] vsprintf: Consistent %pK handling for
 kptr_restrict == 0

On Thu 2018-04-05 08:10:14, Sergey Senozhatsky wrote:
> On (04/04/18 10:58), Petr Mladek wrote:
> >
> > restricted_pointer() pretends that it prints the address when kptr_restrict
> > is set to zero. But it is never called in this situation. Instead,
> > pointer() falls back to ptr_to_id() and hashes the pointer.
> > 
> > This patch removes the potential confusion. klp_restrict is checked only
> > in restricted_pointer().
> > 
> > It should actually fix a small race when the address might get printed
> > unhashed:
> 
> Early morning, didn't have my coffee yet [like really didn't].
> 
> But I don't see how you "fix" a race. "echo 0" might still be called
> later than switch().

I assume that switch() is safe enough and we always have to take one
path there. The old code allowed to print the original pointer without
the capability check (case 0). The new code does:

    case 0: print hashed pointer
    case 1: keep original pointer only with sufficient privileges
    case 2:
    default: set NULL

The old code expected that it would never use case 0. But it was
possible because of the race.


> [..]
> > @@ -1426,8 +1427,8 @@ char *restricted_pointer(char *buf, char *end, const void *ptr,
> >  
> >  	switch (kptr_restrict) {
> >  	case 0:
> > -		/* Always print %pK values */
> > -		break;
> > +		/* Handle as %p, hash and do _not_ leak addresses. */
> > +		return ptr_to_id(buf, end, ptr, spec);
> 
> >From "Always print pK values" to "Always print hashed values"... Do we need
> %pK then? You probably need to update printk-formats.rst as well.

It is already correctly documented in Documentation/sysctl/kernel.txt.
printk-formats.rst uses reference to it.

Best Regards,
Petr

Powered by blists - more mailing lists