linux-kernel - Re: [PATCH 4/4] refcount: Report failures through CHECK_DATA

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: <20170206085739.GH6515@twins.programming.kicks-ass.net>
Date:   Mon, 6 Feb 2017 09:57:39 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     "Reshetova, Elena" <elena.reshetova@...el.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <h.peter.anvin@...el.com>,
        Will Deacon <will.deacon@....com>,
        David Windsor <dwindsor@...il.com>,
        Hans Liljestrand <ishkamiel@...il.com>,
        David Howells <dhowells@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH 4/4] refcount: Report failures through
 CHECK_DATA_CORRUPTION

On Sun, Feb 05, 2017 at 03:33:36PM -0800, Kees Cook wrote:
> On Sun, Feb 5, 2017 at 7:40 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> > On Fri, Feb 03, 2017 at 03:26:52PM -0800, Kees Cook wrote:
> >> This converts from WARN_ON() to CHECK_DATA_CORRUPTION() in the
> >> CONFIG_DEBUG_REFCOUNT case. Additionally moves refcount_t sanity check
> >> conditionals into regular function flow. Since CHECK_DATA_CORRUPTION()
> >> is marked __much_check, we override few cases where the failure has
> >> already been handled but we want to explicitly report it.
> >>
> >> Signed-off-by: Kees Cook <keescook@...omium.org>
> >> ---
> >>  include/linux/refcount.h | 35 ++++++++++++++++++++++-------------
> >>  lib/Kconfig.debug        |  2 ++
> >>  2 files changed, 24 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/include/linux/refcount.h b/include/linux/refcount.h
> >> index 5b89cad62237..ef32910c7dd8 100644
> >> --- a/include/linux/refcount.h
> >> +++ b/include/linux/refcount.h
> >> @@ -43,10 +43,10 @@
> >>  #include <linux/spinlock.h>
> >>
> >>  #if CONFIG_DEBUG_REFCOUNT
> >> -#define REFCOUNT_WARN(cond, str) WARN_ON(cond)
> >> +#define REFCOUNT_CHECK(cond, str) CHECK_DATA_CORRUPTION(cond, str)
> >
> > OK, so that goes back to a full WARN() which will make the generated
> > code gigantic due to the whole printk() trainwreck :/
> 
> Hrm, perhaps we need three levels? WARN_ON, WARN, and BUG?

Did consider that, didn't really know if that made sense.

Like I wrote, ideally we'd end up using something like the x86 exception
table with a custom handler. Just no idea how to pull that off without
doing a full blown arch specific implementation, so I didn't go there
quite yet.


That way refcount_inc() would end up being inlined to something like:

        mov    0x148(%rdi),%eax
        jmp    2f
  1:    lock cmpxchg %edx,0x148(%rdi)
        je     4f
  2:    lea    -0x1(%rax),%ecx
        lea    0x1(%rax),%edx
        cmp    $0xfffffffd,%ecx
        jbe    1b
  3:    ud2
  4:

	_ASM_EXTABLE_HANDLE(3b, 4b, ex_handler_refcount_inc)


where:

bool ex_handler_refcount_inc(const struct exception_table_entry *fixup,
			     struct pt_regs *regs, int trapnr)
{
	regs->ip = ex_fixup_addr(fixup);

	if (!regs->ax)
		WARN(1, "refcount_t: increment on 0; use-after-free.\n");
	else
		WARN(1, "refcount_t: saturated; leaking memory.\n");

	return true;
}

and the handler is shared between all instances and can be as big and
fancy as we'd like.