[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170509193302.f4eqholma2tw2llj@treble>
Date: Tue, 9 May 2017 14:33:02 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Kees Cook <keescook@...omium.org>
Cc: linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
PaX Team <pageexec@...email.hu>, Jann Horn <jannh@...gle.com>,
Eric Biggers <ebiggers3@...il.com>,
Christoph Hellwig <hch@...radead.org>,
"axboe@...nel.dk" <axboe@...nel.dk>,
James Bottomley <James.Bottomley@...senpartnership.com>,
Elena Reshetova <elena.reshetova@...el.com>,
Hans Liljestrand <ishkamiel@...il.com>,
David Windsor <dwindsor@...il.com>,
"x86@...nel.org" <x86@...nel.org>, Ingo Molnar <mingo@...nel.org>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"David S. Miller" <davem@...emloft.net>,
Rik van Riel <riel@...hat.com>,
linux-arch <linux-arch@...r.kernel.org>,
kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH v4 2/2] x86/refcount: Implement fast refcount overflow
protection
On Tue, May 09, 2017 at 12:01:23PM -0700, Kees Cook wrote:
> This protection is a modified version of the x86 PAX_REFCOUNT defense
> from PaX/grsecurity. This speeds up the refcount_t API by duplicating
> the existing atomic_t implementation with a single instruction added to
> detect if the refcount has wrapped past INT_MAX (or below 0) resulting
> in a negative value, where the handler then restores the refcount_t to
> INT_MAX. With this overflow protection, the use-after-free following a
> refcount_t wrap is blocked from happening, avoiding the vulnerability
> entirely.
>
> While this defense only perfectly protects the overflow case, as that
> can be detected and stopped before the reference is freed and left to be
> abused by an attacker, it also notices some of the "inc from 0" and "below
> 0" cases. However, these only indicate that a use-after-free has already
> happened. Such notifications are likely avoidable by an attacker that has
> already exploited a use-after-free vulnerability, but it's better to have
> them than allow such conditions to remain universally silent.
>
> On overflow detection (actually "negative value" detection), the refcount
> value is reset to INT_MAX, the offending process is killed, and a report
> and stack trace are generated. This allows the system to attempt to
> keep operating. Another option, though not done in this patch, would be
> to reset the counter to (INT_MIN / 2) to trap all future refcount inc
> or dec actions, but this would result in even legitimate uses getting
> blocked. Yet another option would be to choose (INT_MAX - N) with some
> small N to provide some headroom for legitimate users of the reference
> counter.
>
> On the matter of races, since the entire range beyond INT_MAX but before 0
> is negative, every inc will trap, leaving no overflow-only race condition.
>
> As for performance, this implementation adds a single "js" instruction to
> the regular execution flow of a copy of the regular atomic_t operations.
> Since this is a forward jump, it is by default the non-predicted path,
> which will be reinforced by dynamic branch prediction. The result is this
> protection having no measurable change in performance over standard
> atomic_t operations. The error path, located in .text.unlikely, uses
> UD0 to fire a refcount exception handler, which reports and returns to
> regular execution. This keeps the changes to .text size minimal, avoiding
> return jumps and open-coded calls to the error reporting routine.
>
> Assembly comparison:
>
> atomic_inc
> .text:
> ffffffff81546149: f0 ff 45 f4 lock incl -0xc(%rbp)
>
> refcount_inc
> .text:
> ffffffff81546149: f0 ff 45 f4 lock incl -0xc(%rbp)
> ffffffff8154614d: 0f 88 80 d5 17 00 js ffffffff816c36d3
> ...
> .text.unlikely:
> ffffffff816c36d3: c7 45 f4 ff ff ff 7f movl $0x7fffffff,-0xc(%rbp)
> ffffffff816c36da: 0f ff (bad)
>
> Various differences from PaX:
> - uses earlier value reset implementation in assembly
> - uses UD0 and refcount exception handler instead of new int vector
> - uses .text.unlikely instead of custom named text sections
> - applied only to refcount_t, not atomic_t (single size, only overflow)
> - reorganized refcount error handler
> - uses "js" instead of "jo" to trap all negative results instead of
> just under/overflow transitions
>
> Signed-off-by: Kees Cook <keescook@...omium.org>
Reviewed-by: Josh Poimboeuf <jpoimboe@...hat.com>
--
Josh
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.
