[<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.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
