On Sun, Feb 05, 2017 at 03:33:36PM -0800, Kees Cook wrote:
> On Sun, Feb 5, 2017 at 7:40 AM, Peter Zijlstra <pet...@infradead.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 <keesc...@chromium.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.
