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.