Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752874AbdFUJ5X (ORCPT ); Wed, 21 Jun 2017 05:57:23 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:33866 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751744AbdFUJ5U (ORCPT ); Wed, 21 Jun 2017 05:57:20 -0400 Date: Wed, 21 Jun 2017 11:57:15 +0200 From: Ingo Molnar To: Kees Cook Cc: linux-kernel@vger.kernel.org, Christoph Hellwig , Peter Zijlstra , "Eric W. Biederman" , Andrew Morton , Josh Poimboeuf , Jann Horn , Eric Biggers , Elena Reshetova , Hans Liljestrand , David Windsor , Greg KH , Ingo Molnar , Alexey Dobriyan , "Serge E. Hallyn" , arozansk@redhat.com, Davidlohr Bueso , Manfred Spraul , "axboe@kernel.dk" , James Bottomley , "x86@kernel.org" , Arnd Bergmann , "David S. Miller" , Rik van Riel , linux-arch Subject: Re: [PATCH v2] refcount: Create unchecked atomic_t implementation Message-ID: <20170621095715.zf57frodvjmhdttg@gmail.com> References: <20170608025831.GA43608@beast> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170608025831.GA43608@beast> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2745 Lines: 72 * Kees Cook wrote: > Many subsystems will not use refcount_t unless there is a way to build the > kernel so that there is no regression in speed compared to atomic_t. This > adds CONFIG_REFCOUNT_FULL to enable the full refcount_t implementation > which has the validation but is slightly slower. When not enabled, > refcount_t uses the basic unchecked atomic_t routines, which results in > no code changes compared to just using atomic_t directly. > > Signed-off-by: Kees Cook > --- > This is v2 of this patch, which I've split from the arch-specific > alternative implementation for x86. Getting this patch in will unblock > atomic_t -> refcount_t conversion, and the x86 alternative implementation > can be developed in parallel. Changes from v1: use better atomic ops, > thanks to Elena and Peter. > --- > arch/Kconfig | 9 +++++++++ > include/linux/refcount.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ > lib/refcount.c | 3 +++ > 3 files changed, 56 insertions(+) Looks almost good - sans a few stylistic nits: > > diff --git a/arch/Kconfig b/arch/Kconfig > index 6c00e5b00f8b..fba3bf186728 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -867,4 +867,13 @@ config STRICT_MODULE_RWX > config ARCH_WANT_RELAX_ORDER > bool > > +config REFCOUNT_FULL > + bool "Perform full reference count validation at the expense of speed" > + help > + Enabling this switches the refcounting infrastructure from a fast > + unchecked atomic_t implementation to a fully state checked > + implementation, which can be slower but provides protections > + against various use-after-free conditions that can be used in > + security flaw exploits. > + > source "kernel/gcov/Kconfig" > diff --git a/include/linux/refcount.h b/include/linux/refcount.h > index b34aa649d204..099c32bd07b2 100644 > --- a/include/linux/refcount.h > +++ b/include/linux/refcount.h > @@ -41,6 +41,7 @@ static inline unsigned int refcount_read(const refcount_t *r) > return atomic_read(&r->refs); > } > > +#ifdef CONFIG_REFCOUNT_FULL > extern __must_check bool refcount_add_not_zero(unsigned int i, refcount_t *r); > extern void refcount_add(unsigned int i, refcount_t *r); > > @@ -52,6 +53,49 @@ extern void refcount_sub(unsigned int i, refcount_t *r); > > extern __must_check bool refcount_dec_and_test(refcount_t *r); > extern void refcount_dec(refcount_t *r); > +#else > +static inline __must_check bool refcount_add_not_zero(unsigned int i, > + refcount_t *r) Please keep it on a single, slighly over-long line instead of the ugly line break in the middle of the list of parameters ... There's other such uglies in the patch as well. Thanks, Ingo